[multistage] Add Pushdown and Worker Rules #15658
Conversation
ankitsultana
added
multi-stage
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #15658 +/- ##
============================================
- Coverage 62.79% 62.66% -0.14%
Complexity 1384 1384
============================================
Files 2865 2869 +4
Lines 162973 163514 +541
Branches 24931 25060 +129
============================================
+ Hits 102339 102460 +121
- Misses 52910 53316 +406
- Partials 7724 7738 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ankitsultana
force-pushed
the
mse-physical-p4.3
branch
from
93d1e51 to
1cf9edd
Compare
| throw new IllegalArgumentException("Unsupported collection type: " + relDataType); | ||
| } | ||
| LOGGER.warn("Unexpected SQL type: {}, use OBJECT instead", sqlTypeName); | ||
| return ColumnDataType.OBJECT; |
There was a problem hiding this comment.
What falls under this case?
There was a problem hiding this comment.
This is taken as is from RelToPlanNodeConverter.
I think this is just to be future compatible. We may add more types to Pinot but would likely miss out on covering them here. This way we'll log a warning and can retroactively fix the case handling here.
| aggPRelNode.getLimit(), idGenerator); | ||
| } | ||
|
|
||
| // TODO: Currently it only handles one WITHIN GROUP collation across all AggregateCalls. |
There was a problem hiding this comment.
what's the failure mode when there are multiple collations? should we have some precondition?
There was a problem hiding this comment.
Good catch. Similar to the other comment, this is taken as is from existing code:
Ideally we should throw an error but not sure if we do that right now. cc: @Jackie-Jiang
3 participants
Note: Does NOT touch any existing multistage code path and only contains changes to the new optimizer.
Summary
Adds Sort / Aggregate Pushdown and Worker Assignment rules. Also adds the
PRelToPlanNodeConverterwhich is taken as is from theRelToPlanNodeConverter. I have commented out the Spooling and metrics related code for now and will be fixing them in a future PR as part of productionization of the Physical Optimizer.Sort Pushdown
Sort pushdown pushes a Sort past an Exchange when there's a non-null "fetch" set in the Sort. The value of the fetch is computed using "fetch + offset" value of the top-level sort.
Note: There are possibilities to do more kinds of pushdown here. For instance, if there's a fetch only Sort right above a Join, then we could annotate the Join with the fetch limit and remove the Sort.
Aggregate Pushdown
This is similar to the existing
PinotAggregateNodeExchangeInsertRule.Worker Assignment
Worker assignment is where Exchanges are simplified, allowing for full plan colocation. I have added some details in the Javadoc of the corresponding class.
Test Plan
We have the full Physical Optimizer changes running in a test cluster, and the next PR will also add Integration Tests. You can see a preview of them here: https://github.com/ankitsultana/pinot/pull/46/files#diff-25751d60157478f9d864a75c86bf6902f7870c879640887741e59d17e0162643