Commit 10db6b3
Redesign the try_reverse_output to support more cases (#19446)
## Which issue does this PR close?
- Closes #19403
## Rationale for this change
After #19064 was completed, I found it couldn't meet our internal
project requirements:
1. **Redesign try_reverse_output using EquivalenceProperties**
- The previous implementation used a simple `is_reverse` method that
could only handle basic reverse matching
- Now leveraging `EquivalenceProperties` can handle more complex
scenarios, including constant column elimination and monotonic functions
2. **Switch to ordering_satisfy method for ordering matching**
- This method internally:
- Normalizes orderings (removes constant columns)
- Checks monotonic functions (like `date_trunc`, `CAST`, `CEIL`)
- Handles prefix matching
3. **Extend sort pushdown support to more operators**
- Added `try_pushdown_sort` implementation for `ProjectionExec`,
`FilterExec`, `CooperativeExec`
- These operators can now pass sort requests down to their children
## What changes are included in this PR?
### Core Changes:
1. **ParquetSource::try_reverse_output**
(datasource-parquet/src/source.rs)
- Added `eq_properties` parameter
- Reverses all orderings in equivalence properties
- Uses `ordering_satisfy` to check if reversed ordering satisfies the
request
- Removed `file_ordering` field and `with_file_ordering_info` method
2. **FileSource trait** (datasource/src/file.rs)
- Updated `try_reverse_output` signature with `eq_properties` parameter
- Added detailed documentation explaining parameter usage and examples
3. **FileScanConfig::try_pushdown_sort**
(datasource/src/file_scan_config.rs)
- Simplified logic to directly call `file_source.try_reverse_output`
- No longer needs to pre-check ordering satisfaction or set file
ordering info
4. **New operator support**
- `FilterExec::try_pushdown_sort` - Pushes sort below filters
- `ProjectionExec::try_pushdown_sort` - Pushes sort below projections
- `CooperativeExec::try_pushdown_sort` - Supports sort pushdown in
cooperative execution
5. **Removed obsolete methods**
- Deleted `LexOrdering::is_reverse` - replaced by `ordering_satisfy`
### Feature Enhancements:
**Supported optimization scenarios:**
1. **Constant column elimination** (Test 7)
```sql
-- File ordering: [timeframe ASC, period_end ASC]
-- Query: WHERE timeframe = 'quarterly' ORDER BY period_end DESC
-- Effect: After timeframe becomes constant, reverse scan is enabled
```
2. **Monotonic function support** (Test 8)
```sql
-- File ordering: [ts ASC]
-- Query: ORDER BY date_trunc('month', ts) DESC
-- Effect: date_trunc is monotonic, reverse scan satisfies the request
```
## Are these changes tested?
Yes, comprehensive tests have been added:
- **Test 7 (237 lines)**: Constant column elimination scenarios
- Single constant column filter
- Multi-value IN clauses (doesn't trigger optimization)
- Literal constants in sort expressions
- Non-leading column filters (edge cases)
- **Test 8 (355 lines)**: Monotonic function scenarios
- `date_trunc` (date truncation)
- `CAST` (type conversion)
- `CEIL` (ceiling)
- `ABS` (negative case - not monotonic over mixed positive/negative
range)
All tests verify:
- Presence of `reverse_row_groups=true` in physical plans
- Correctness of query results
## Are there any user-facing changes?
**API Changes:**
- `FileSource::try_reverse_output` signature changed (added
`eq_properties` parameter)
- Removed `FileSource::with_file_ordering_info` method
- Removed `LexOrdering::is_reverse` public method
**User-visible improvements:**
- More queries can leverage reverse row group scanning for optimization
- Especially queries with `WHERE` clauses that make certain columns
constant
- Queries using monotonic functions (like date functions, type
conversions)
**Note:** This PR returns `Inexact` results because only row group order
is reversed, not row order within row groups. Future enhancements could
include:
- File reordering based on statistics (returning `Exact`)
- Partial sort pushdown for prefix matches
---------
Co-authored-by: Adrian Garcia Badaracco <[email protected]>1 parent 4960284 commit 10db6b3
File tree
9 files changed
+1646
-412
lines changed- datafusion
- core/tests/physical_optimizer
- datasource-parquet/src
- datasource/src
- physical-plan/src
- sqllogictest/test_files
9 files changed
+1646
-412
lines changedLines changed: 374 additions & 6 deletions
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
41 | 41 | | |
42 | 42 | | |
43 | 43 | | |
| 44 | + | |
44 | 45 | | |
45 | 46 | | |
46 | 47 | | |
| |||
54 | 55 | | |
55 | 56 | | |
56 | 57 | | |
| 58 | + | |
57 | 59 | | |
58 | 60 | | |
59 | 61 | | |
| |||
68 | 70 | | |
69 | 71 | | |
70 | 72 | | |
71 | | - | |
| 73 | + | |
72 | 74 | | |
73 | 75 | | |
74 | 76 | | |
| |||
774 | 776 | | |
775 | 777 | | |
776 | 778 | | |
| 779 | + | |
| 780 | + | |
| 781 | + | |
| 782 | + | |
| 783 | + | |
| 784 | + | |
| 785 | + | |
| 786 | + | |
| 787 | + | |
| 788 | + | |
| 789 | + | |
| 790 | + | |
| 791 | + | |
| 792 | + | |
| 793 | + | |
| 794 | + | |
| 795 | + | |
| 796 | + | |
| 797 | + | |
| 798 | + | |
| 799 | + | |
| 800 | + | |
| 801 | + | |
| 802 | + | |
| 803 | + | |
| 804 | + | |
| 805 | + | |
| 806 | + | |
| 807 | + | |
| 808 | + | |
| 809 | + | |
| 810 | + | |
| 811 | + | |
| 812 | + | |
| 813 | + | |
| 814 | + | |
| 815 | + | |
| 816 | + | |
| 817 | + | |
| 818 | + | |
| 819 | + | |
| 820 | + | |
| 821 | + | |
| 822 | + | |
| 823 | + | |
| 824 | + | |
| 825 | + | |
| 826 | + | |
| 827 | + | |
| 828 | + | |
| 829 | + | |
| 830 | + | |
| 831 | + | |
| 832 | + | |
| 833 | + | |
| 834 | + | |
| 835 | + | |
| 836 | + | |
| 837 | + | |
| 838 | + | |
| 839 | + | |
| 840 | + | |
| 841 | + | |
| 842 | + | |
| 843 | + | |
| 844 | + | |
| 845 | + | |
| 846 | + | |
| 847 | + | |
| 848 | + | |
| 849 | + | |
| 850 | + | |
| 851 | + | |
| 852 | + | |
| 853 | + | |
| 854 | + | |
| 855 | + | |
| 856 | + | |
| 857 | + | |
| 858 | + | |
| 859 | + | |
| 860 | + | |
| 861 | + | |
| 862 | + | |
| 863 | + | |
| 864 | + | |
| 865 | + | |
| 866 | + | |
| 867 | + | |
| 868 | + | |
| 869 | + | |
| 870 | + | |
| 871 | + | |
| 872 | + | |
| 873 | + | |
| 874 | + | |
| 875 | + | |
| 876 | + | |
| 877 | + | |
| 878 | + | |
| 879 | + | |
| 880 | + | |
| 881 | + | |
| 882 | + | |
| 883 | + | |
| 884 | + | |
| 885 | + | |
| 886 | + | |
| 887 | + | |
| 888 | + | |
| 889 | + | |
| 890 | + | |
| 891 | + | |
| 892 | + | |
| 893 | + | |
| 894 | + | |
| 895 | + | |
| 896 | + | |
| 897 | + | |
| 898 | + | |
| 899 | + | |
| 900 | + | |
| 901 | + | |
| 902 | + | |
| 903 | + | |
| 904 | + | |
| 905 | + | |
| 906 | + | |
| 907 | + | |
| 908 | + | |
| 909 | + | |
| 910 | + | |
| 911 | + | |
| 912 | + | |
| 913 | + | |
| 914 | + | |
| 915 | + | |
| 916 | + | |
| 917 | + | |
| 918 | + | |
| 919 | + | |
| 920 | + | |
| 921 | + | |
| 922 | + | |
| 923 | + | |
| 924 | + | |
| 925 | + | |
| 926 | + | |
| 927 | + | |
| 928 | + | |
| 929 | + | |
| 930 | + | |
| 931 | + | |
| 932 | + | |
| 933 | + | |
| 934 | + | |
| 935 | + | |
| 936 | + | |
| 937 | + | |
| 938 | + | |
| 939 | + | |
| 940 | + | |
| 941 | + | |
| 942 | + | |
| 943 | + | |
| 944 | + | |
| 945 | + | |
| 946 | + | |
| 947 | + | |
| 948 | + | |
| 949 | + | |
| 950 | + | |
| 951 | + | |
| 952 | + | |
| 953 | + | |
| 954 | + | |
| 955 | + | |
| 956 | + | |
| 957 | + | |
| 958 | + | |
| 959 | + | |
| 960 | + | |
| 961 | + | |
| 962 | + | |
| 963 | + | |
| 964 | + | |
| 965 | + | |
| 966 | + | |
| 967 | + | |
| 968 | + | |
| 969 | + | |
| 970 | + | |
| 971 | + | |
| 972 | + | |
| 973 | + | |
| 974 | + | |
| 975 | + | |
| 976 | + | |
| 977 | + | |
| 978 | + | |
| 979 | + | |
| 980 | + | |
| 981 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
41 | | - | |
42 | 41 | | |
| 42 | + | |
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
| |||
54 | 54 | | |
55 | 55 | | |
56 | 56 | | |
57 | | - | |
| 57 | + | |
58 | 58 | | |
59 | 59 | | |
60 | 60 | | |
| |||
288 | 288 | | |
289 | 289 | | |
290 | 290 | | |
291 | | - | |
292 | | - | |
293 | | - | |
294 | 291 | | |
295 | 292 | | |
296 | 293 | | |
| |||
320 | 317 | | |
321 | 318 | | |
322 | 319 | | |
323 | | - | |
324 | 320 | | |
325 | 321 | | |
326 | 322 | | |
| |||
397 | 393 | | |
398 | 394 | | |
399 | 395 | | |
400 | | - | |
401 | | - | |
402 | | - | |
403 | | - | |
404 | | - | |
405 | | - | |
406 | 396 | | |
407 | 397 | | |
408 | 398 | | |
| |||
769 | 759 | | |
770 | 760 | | |
771 | 761 | | |
| 762 | + | |
772 | 763 | | |
773 | | - | |
774 | | - | |
775 | | - | |
776 | | - | |
777 | | - | |
778 | | - | |
779 | | - | |
780 | | - | |
781 | | - | |
| 764 | + | |
782 | 765 | | |
783 | | - | |
| 766 | + | |
784 | 767 | | |
785 | | - | |
786 | | - | |
787 | | - | |
788 | | - | |
| 768 | + | |
| 769 | + | |
| 770 | + | |
| 771 | + | |
| 772 | + | |
| 773 | + | |
| 774 | + | |
| 775 | + | |
| 776 | + | |
| 777 | + | |
| 778 | + | |
| 779 | + | |
| 780 | + | |
| 781 | + | |
| 782 | + | |
| 783 | + | |
| 784 | + | |
| 785 | + | |
| 786 | + | |
| 787 | + | |
| 788 | + | |
| 789 | + | |
| 790 | + | |
| 791 | + | |
| 792 | + | |
| 793 | + | |
| 794 | + | |
| 795 | + | |
| 796 | + | |
| 797 | + | |
| 798 | + | |
| 799 | + | |
789 | 800 | | |
790 | | - | |
791 | | - | |
792 | | - | |
793 | | - | |
794 | | - | |
| 801 | + | |
| 802 | + | |
| 803 | + | |
795 | 804 | | |
796 | 805 | | |
| 806 | + | |
| 807 | + | |
| 808 | + | |
| 809 | + | |
| 810 | + | |
| 811 | + | |
| 812 | + | |
797 | 813 | | |
798 | 814 | | |
799 | 815 | | |
800 | 816 | | |
801 | | - | |
802 | | - | |
803 | | - | |
804 | | - | |
805 | | - | |
806 | | - | |
807 | | - | |
808 | | - | |
809 | | - | |
810 | 817 | | |
811 | 818 | | |
812 | 819 | | |
| |||
0 commit comments