FrankChen021 commented on code in PR #20314:
URL: https://github.com/apache/druid/pull/20314#discussion_r4001420398
##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java:
##########
@@ -303,12 +308,45 @@ private Program
buildDecoupledLogicalOptimizationProgram(PlannerContext plannerC
cleanupRules.addRuleInstance(CoreRules.PROJECT_MERGE);
cleanupRules.addRuleInstance(AggregateProjectMergeRule.Config.DEFAULT.toRule());
return Programs.sequence(
- Programs.of(builder.build(), true,
DefaultRelMetadataProvider.INSTANCE),
+ hepProgram(builder.build(), plannerContext),
new DruidTrimFieldsProgram(),
- Programs.of(cleanupRules.build(), true,
DefaultRelMetadataProvider.INSTANCE)
+ hepProgram(cleanupRules.build(), plannerContext)
);
}
+ /**
+ * Equivalent to {@link Programs#of(HepProgram, boolean,
org.apache.calcite.rel.metadata.RelMetadataProvider)} with
+ * {@code noDag=true} and {@link DefaultRelMetadataProvider}, except the
{@link HepPlanner} is given a
+ * {@link org.apache.calcite.plan.Context} carrying the per-query {@link
org.apache.calcite.util.CancelFlag}.
+ * {@code Programs.of} builds its {@code HepPlanner} with a {@code null}
context, so it never observes our cancel
+ * flag; a query that spends its planning budget in these Hep rule loops
would otherwise ignore the planning timeout.
+ */
+ private static Program hepProgram(final HepProgram hepProgram, final
PlannerContext plannerContext)
+ {
+ return (planner, rel, requiredOutputTraits, materializations, lattices) ->
{
+ final HepPlanner hepPlanner = new HepPlanner(
+ hepProgram,
+ Contexts.of(plannerContext.getCancelFlag()),
+ true,
+ null,
+ RelOptCostImpl.FACTORY
+ );
+ for (final RelOptMaterialization materialization : materializations) {
+ hepPlanner.addMaterialization(materialization);
+ }
+ for (final RelOptLattice lattice : lattices) {
+ hepPlanner.addLattice(lattice);
+ }
+ // Modern Calcite routes rule metadata through the cluster's provider
(the planner-level
+ // registerMetadataProviders is deprecated), so set it on the cluster as
Programs.of does.
+ final List<RelMetadataProvider> metadataProviders = new ArrayList<>();
+ metadataProviders.add(DefaultRelMetadataProvider.INSTANCE);
+
rel.getCluster().setMetadataProvider(ChainedRelMetadataProvider.of(metadataProviders));
Review Comment:
[P2] Preserve Calcite's Hep metadata provider
**Finding:** Calcite 1.42's Programs.of still calls
HepPlanner.registerMetadataProviders before chaining the providers into the
cluster, and HepPlanner overrides that deprecated method to prepend
HepRelMetadataProvider for metadata requests involving HepRelVertex. This
replacement builds the chain with only DefaultRelMetadataProvider, so the
pre/reduction/cleanup stages no longer reproduce Programs.of and can lose or
fail metadata lookups when a Hep wrapper is involved.
**Suggestion:** Retain the HepPlanner registration call despite its
deprecation, or otherwise include the Hep-specific metadata provider in the
chain before installing it on the cluster.
--
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]
