snuyanzin commented on code in PR #5031: URL: https://github.com/apache/calcite/pull/5031#discussion_r3524654655
########## core/src/main/java/org/apache/calcite/rel/rules/CorrelateUncollectMergeRule.java: ########## @@ -0,0 +1,204 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.calcite.rel.rules; + +import org.apache.calcite.plan.RelOptRuleCall; +import org.apache.calcite.plan.RelOptUtil; +import org.apache.calcite.plan.RelRule; +import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.core.Correlate; +import org.apache.calcite.rel.core.CorrelationId; +import org.apache.calcite.rel.core.JoinRelType; +import org.apache.calcite.rel.core.Project; +import org.apache.calcite.rel.core.Uncollect; +import org.apache.calcite.rel.logical.LogicalValues; +import org.apache.calcite.rel.type.RelDataTypeField; +import org.apache.calcite.rex.RexBuilder; +import org.apache.calcite.rex.RexCorrelVariable; +import org.apache.calcite.rex.RexFieldAccess; +import org.apache.calcite.rex.RexInputRef; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.rex.RexShuttle; +import org.apache.calcite.tools.RelBuilder; +import org.apache.calcite.util.ImmutableBitSet; +import org.apache.calcite.util.Util; + +import org.immutables.value.Value; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * Rule that converts a {@link Correlate} over an {@link Uncollect} into a + * single, generalized {@link Uncollect} that reads directly from the + * correlate's left input, when possible. + * + * <p>Input plan: + * <pre> + * LogicalProject (outerProject) + * LogicalCorrelate(cor=[$cor0], joinType=[inner|left|...]) + * left (any RelNode) + * Uncollect + * LogicalProject($cor0.f_i, $cor0.f_j, ...) (innerProject) + * LogicalValues + * </pre> + * + * <p>Converted to: + * <pre> + * LogicalProject (outerProject, remapped to the merged Uncollect's output indices) + * Uncollect(collectionFields=[f_i, f_j, ...]) + * left + * </pre> + * + * <p>The rule fires only when every expression in {@code innerProject} is a + * direct field access on the correlation variable (no nested struct paths). + * Collection fields referenced in {@code outerProject} are passed through + * as raw values alongside their element-column expansion. + * + * <p>This rule is a generalization of {@link UnnestDecorrelateRule}. + * + * <p>This merge cannot be performed by + * {@link org.apache.calcite.sql2rel.SqlToRelConverter}, which builds the {@code Correlate} + * over {@code Uncollect} shape. During conversion, each FROM-item alias ({@code t1} and + * {@code t2} in {@code SELECT t2.x FROM t1, UNNEST(t1.arr) AS t2(x)}) is resolved to a + * column range by locating a distinct sub-node of the plan for its namespace. A + * {@code Correlate} has one child per namespace, but the merged {@code Uncollect} has a + * single input, so references to {@code t2} converted after the merge (the SELECT list, + * for example) would find no node to resolve to. A planner rule fires after conversion, + * when every reference is already a fixed {@link RexInputRef} offset. + */ [email protected] +public class CorrelateUncollectMergeRule + extends RelRule<CorrelateUncollectMergeRule.Config> + implements TransformationRule { + + protected CorrelateUncollectMergeRule(Config config) { + super(config); + } + + @Override public void onMatch(RelOptRuleCall call) { Review Comment: I played locally and seems found a case where current implementation fails ```java final String sql = "with t1 as (select array['a', 'b'] as sa, array[1, 2] as ia)\n" + "select u.x, u.y\n" + "from t1, unnest(t1.ia, t1.sa) as u(x, y)"; sql(sql) .withPreRule(CoreRules.PROJECT_REMOVE) .withRule(CoreRules.CORRELATE_UNCOLLECT_MERGE) .check(); ``` currently fails as ``` java.lang.AssertionError: Cannot add expression of different type to set: set type is RecordType(INTEGER X, CHAR(1) Y) NOT NULL expression type is RecordType(CHAR(1) SA, INTEGER IA) NOT NULL set is rel#302523:LogicalProject.(input=HepRelVertex#302522,exprs=[$2, $3]) expression is Uncollect LogicalProject(SA=[ARRAY('a', 'b')], IA=[ARRAY(1, 2)]) LogicalValues(tuples=[[{ 0 }]]) Type mismatch: rowtype of original rel: RecordType(INTEGER X, CHAR(1) Y) NOT NULL rowtype of new rel: RecordType(CHAR(1) SA, INTEGER IA) NOT NULL Difference: X: INTEGER -> CHAR(1) Y: CHAR(1) -> INTEGER ``` -- 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]
