gortiz opened a new pull request, #19033: URL: https://github.com/apache/pinot/pull/19033
## Summary Enriched joins were an experimental feature (introduced in #16123 by @songwdfu — thanks for the original contribution) that fused project/filter/sort into the hash join operator to try to improve performance. In practice they did not deliver the expected gains, so this PR removes the feature. The removal is deliberately **backward compatible**: the query option becomes a no-op rather than an error, and the wire/plan-node/visitor surface is kept and `@Deprecated` rather than deleted, so a plan serialized by an older-version broker still deserializes during a rolling upgrade. ## What was removed - `PinotEnrichedJoinRule`, `PinotLogicalEnrichedJoin` (Calcite rel), and `EnrichedHashJoinOperator`, plus their tests and the `EnrichedJoin.json` resource. - The rule wiring in `QueryEnvironment`: the `JoinToEnrichedJoin` rule is no longer registered, so a query requesting it via `usePlannerRules` is **silently ignored** instead of failing. - The always-null enriched `filterProjectRexNodes` field/constructor on the v2 `PhysicalJoin`. ## What was retained (`@Deprecated`, for backward compatibility) - `EnrichedJoinNode` plan node, `PlanNodeVisitor#visitEnrichedJoin` and all its overrides, the serde path, `plan.proto` field 17 (retained, **not reused**), and the `CommonConstants.JOIN_TO_ENRICHED_JOIN` constant. - `DefaultJoinOperatorFactory#createEnrichedJoinOperator` now throws `UnsupportedOperationException`, so if an older broker's plan carrying an `EnrichedJoinNode` reaches an upgraded server it **fails loudly** rather than silently executing a degraded join. - A sunset note in the `EnrichedJoinNode` javadoc documents when the skeleton can be fully deleted (once no supported broker version still emits proto field 17). ## Compatibility notes - **SPI**: `pinot-spi` `CommonConstants.JOIN_TO_ENRICHED_JOIN` is only annotated `@Deprecated`; its string value is unchanged, so existing `usePlannerRules` / `DEFAULT_DISABLED_RULES` references keep working. - **Rolling upgrade**: since enriched joins were experimental and disabled by default, the fail-loud behavior on the server for a mixed-version plan is intentional. ## Testing - New `PlanNodeSerDeTest#testEnrichedJoinNodeSerDe` — direct serde round-trip guarding the retained wire format (asserts the enriched-specific fields explicitly, since `JoinNode#equals` ignores them). - New `QueryCompilationTest#testEnrichedJoinRuleIsNoOp` — verifies `usePlannerRules='JoinToEnrichedJoin'` plans successfully as an ordinary join with no enriched node. - New `DefaultJoinOperatorFactoryTest` — verifies the runtime rejects an enriched join. - Existing `ResourceBasedQueryPlansTest`, `RelToPlanNodeConverterTest`, `PlanNodeSerDeTest`, `QueryCompilationTest`, `HashJoinOperatorTest` pass. Checkstyle, spotless, and license checks pass. -- 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]
