gortiz commented on code in PR #19174:
URL: https://github.com/apache/pinot/pull/19174#discussion_r3812402122
##########
pinot-query-planner/src/test/resources/queries/ExplainPhysicalPlans.json:
##########
@@ -421,21 +421,15 @@
"\n ├── [2]@localhost:1|[1] PROJECT",
"\n │ └── [2]@localhost:1|[1] TABLE SCAN
(a) null",
"\n └── [2]@localhost:1|[1]
MAIL_RECEIVE(BROADCAST_DISTRIBUTED)",
- "\n ├── [3]@localhost:2|[2]
MAIL_SEND(BROADCAST_DISTRIBUTED)->{[2]@localhost:1|[0, 1],[2]@localhost:2|[2,
3]} (Subtree Omitted)",
- "\n ├── [3]@localhost:2|[3]
MAIL_SEND(BROADCAST_DISTRIBUTED)->{[2]@localhost:1|[0, 1],[2]@localhost:2|[2,
3]} (Subtree Omitted)",
- "\n ├── [3]@localhost:1|[0]
MAIL_SEND(BROADCAST_DISTRIBUTED)->{[2]@localhost:1|[0, 1],[2]@localhost:2|[2,
3]} (Subtree Omitted)",
- "\n └── [3]@localhost:1|[1]
MAIL_SEND(BROADCAST_DISTRIBUTED)->{[2]@localhost:1|[0, 1],[2]@localhost:2|[2,
3]}",
- "\n └── [3]@localhost:1|[1] PROJECT",
- "\n └── [3]@localhost:1|[1]
FILTER",
- "\n └── [3]@localhost:1|[1]
AGGREGATE_FINAL",
- "\n └──
[3]@localhost:1|[1] MAIL_RECEIVE(HASH_DISTRIBUTED)",
- "\n ├──
[4]@localhost:2|[2]
MAIL_SEND(HASH_DISTRIBUTED)[PARTITIONED]->{[3]@localhost:2|[2]} (Subtree
Omitted)",
- "\n ├──
[4]@localhost:2|[3]
MAIL_SEND(HASH_DISTRIBUTED)[PARTITIONED]->{[3]@localhost:2|[3]} (Subtree
Omitted)",
- "\n ├──
[4]@localhost:1|[0]
MAIL_SEND(HASH_DISTRIBUTED)[PARTITIONED]->{[3]@localhost:1|[0]} (Subtree
Omitted)",
- "\n └──
[4]@localhost:1|[1]
MAIL_SEND(HASH_DISTRIBUTED)[PARTITIONED]->{[3]@localhost:1|[1]}",
- "\n └──
[4]@localhost:1|[1] AGGREGATE_LEAF",
- "\n └──
[4]@localhost:1|[1] FILTER",
- "\n └──
[4]@localhost:1|[1] TABLE SCAN (b) null",
+ "\n └── [3]@localhost:1|[0]
MAIL_SEND(BROADCAST_DISTRIBUTED)->{[2]@localhost:1|[0, 1],[2]@localhost:2|[2,
3]}",
+ "\n └── [3]@localhost:1|[0] PROJECT",
+ "\n └── [3]@localhost:1|[0]
FILTER",
+ "\n └── [3]@localhost:1|[0]
AGGREGATE_FINAL",
+ "\n └──
[3]@localhost:1|[0] MAIL_RECEIVE(HASH_DISTRIBUTED)",
+ "\n └──
[4]@localhost:1|[0]
MAIL_SEND(HASH_DISTRIBUTED)[PARTITIONED]->{[3]@localhost:1|[0]}",
Review Comment:
This expectation moving is worth a line in the description, because the
reduction here isn't actually a pruning verdict. `FakeRoutingManager` doesn't
override `getPrunedSegments`, so it returns the interface default `Set.of()` —
"a filter ran and proved nothing" — and `QueryEnvironmentTestBase` builds
partitions 1..3 of `b` as non-null `PartitionInfo`s with empty segment lists,
so `allSegmentsPruned` is vacuously true for them and only class 0 survives.
The new plan is correct — those classes hold no rows, and the old one spent
three workers on nothing. Two things follow:
- Production can't reach that state: `SegmentPartitionMetadataManager` only
creates a partition entry together with its first segment. So
`populatedClasses` (non-null `PartitionInfo`) and `matchingClasses` (some
segment not provably pruned) agree everywhere *except* in this fixture. A
sentence on `markClassesWithMatchingData` noting the two use different notions
of "holds data" would save the next reader the trip.
- This file is the only place in the diff where a single-partitioned-leaf
group — a leaf plus its 1-to-1 parent, no join — shows up being reduced, and it
does so for a fixture reason. Since `toReducibleGroup` needs only one
partitioned leaf, plain partitioned aggregations feeding a pre-partitioned
exchange are in scope too, which the current description ("colocated joins")
undersells.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
