raghavyadav01 opened a new pull request, #19862: URL: https://github.com/apache/pinot/pull/19862
`PinotJoinExchangeNodeInsertRule` turns `left_distribution_type` / `right_distribution_type` into the exchanges that route each side's rows, and carries a `// TODO: Validate if the configured distribution types are valid`. Until it does, several pairs a user can ask for are accepted and answered wrong, with no error and no warning. Measured against H2 on the two-worker `ResourceBasedQueriesTest` setup, with the hint as the only difference between the query and a correct one: | hint | join | H2 | returned | | --- | --- | ---: | ---: | | `random` + `random` | inner, equi | 10 | 8 | | `hash` + `random` | inner, equi | 10 | 2 | | `broadcast` + `broadcast` | inner, equi | 10 | 20 | | `broadcast` on the left | LEFT, equi | 13 | 23 | | `broadcast` on the right | RIGHT, equi | 11 | 19 | | `random` + `broadcast` | RIGHT, non-equi | 39 | 47 | Two conditions explain all six: 1. A pair of rows that could match has to meet on one worker, and each joined row has to be emitted once. Broadcasting one side gives every worker all of its rows, so it stands in for any partitioning of the other. Broadcasting both gives every worker all of both, and the join result repeats once per worker. Broadcasting neither leaves both sides partitioned, which lines up only when both are partitioned the same way on the join keys — and that needs keys to partition on. 2. An outer join also emits the rows of its outer side that matched nothing, which a worker can only determine once it holds every row of the opposite side. So the broadcast side must be the side opposite the outer one. A FULL join is outer on both sides and cannot have a broadcast side at all. This PR replaces the TODO with a check of the resolved pair against both conditions, throwing at plan time with the reason. The pairs `inferLeftDistributionType` / `inferRightDistributionType` produce already satisfy them — including the RIGHT-join inversion they document — so a query that does not hint a distribution type plans exactly as before. The comment above `inferLeftDistributionType` already notes that `RANDOM(left) + BROADCAST(right)` is incorrect for a non-equi RIGHT JOIN, which is why the inference inverts it. The last row of the table is that same combination supplied by hand, where nothing inverted it. ### Testing The six combinations above are added to `QueryHints.json` with the error they must now raise, plus the no-equi-keys case with neither side broadcast, and `LEFT JOIN` with the right side broadcast as the workable direction that must keep planning. Each one returns the wrong row count on master and fails to plan with this change. All 3901 cases in `ResourceBasedQueriesTest` and 2665 in `pinot-query-planner` pass, so no existing plan or result moves. The new error cases carry `ignoreV2Optimizer`, because the physical optimizer does not build its exchanges through this rule and already answers all six correctly — this is the old planner path only. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
