github-advanced-security[bot] commented on code in PR #17177: URL: https://github.com/apache/druid/pull/17177#discussion_r1778687320
########## sql/src/main/java/org/apache/druid/sql/calcite/rule/logical/LogicalUnnestRule.java: ########## @@ -0,0 +1,160 @@ +/* + * 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.druid.sql.calcite.rule.logical; + +import com.google.common.collect.Iterables; +import org.apache.calcite.plan.RelOptRule; +import org.apache.calcite.plan.RelOptRuleCall; +import org.apache.calcite.plan.RelOptUtil; +import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.core.Filter; +import org.apache.calcite.rel.core.Project; +import org.apache.calcite.rel.core.Uncollect; +import org.apache.calcite.rel.logical.LogicalCorrelate; +import org.apache.calcite.rel.logical.LogicalValues; +import org.apache.calcite.rel.rules.SubstitutionRule; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.tools.RelBuilder; +import org.apache.druid.error.DruidException; +import org.apache.druid.sql.calcite.rel.DruidCorrelateUnnestRel; + +/** + * Recognizes a LogicalUnnest operation in the plan. + * + * Matches on the layout: + * + * <pre> + * LogicalCorrelate(correlation=[$cor0], joinType=[inner], requiredColumns=[{4}]) + * RelNodeSubtree + * Uncollect + * LogicalProject(arrayLongNulls=[$cor0.arrayLongNulls]) + * LogicalValues(tuples=[[{ 0 }]]) + * </pre> + * + * Translates it to use a {@link LogicalUnnest} like: + * + * <pre> + * LogicalUnnest(unnestExpr=[$cor0.arrayLongNulls]) + * RelNodeSubtree + * </pre> + * + * It raises an error for cases when {@link LogicalCorrelate} can't be + * translated as those are currently unsupported in Druid. + */ +public class LogicalUnnestRule extends RelOptRule implements SubstitutionRule +{ + public LogicalUnnestRule() + { + super(operand(LogicalCorrelate.class, any())); Review Comment: ## Deprecated method or constructor invocation Invoking [RelOptRule.operand](1) should be avoided because it has been deprecated. [Show more details](https://github.com/apache/druid/security/code-scanning/8354) ########## sql/src/main/java/org/apache/druid/sql/calcite/rule/logical/UnnestInputCleanupRule.java: ########## @@ -0,0 +1,144 @@ +/* + * 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.druid.sql.calcite.rule.logical; + +import org.apache.calcite.plan.RelOptRule; +import org.apache.calcite.plan.RelOptRuleCall; +import org.apache.calcite.plan.RelOptUtil.InputFinder; +import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.core.Project; +import org.apache.calcite.rel.rules.SubstitutionRule; +import org.apache.calcite.rex.RexInputRef; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.rex.RexShuttle; +import org.apache.calcite.util.ImmutableBitSet; +import org.apache.druid.error.DruidException; +import java.util.ArrayList; +import java.util.List; + +/** + * Makes tweaks to LogicalUnnest input. + * + * Removes any MV_TO_ARRAY call if its present for the input of the + * {@link LogicalUnnest}. + * + */ +public class UnnestInputCleanupRule extends RelOptRule implements SubstitutionRule +{ + public UnnestInputCleanupRule() + { + super( + operand( + LogicalUnnest.class, + operand(Project.class, any()) + ) + ); + } + + @Override + public void onMatch(RelOptRuleCall call) + { + LogicalUnnest unnest = call.rel(0); + Project project = call.rel(1); + + ImmutableBitSet input = InputFinder.analyze(unnest.unnestExpr).build(); + if (input.isEmpty()) { + throw DruidException.defensive("Found an unbound unnest expression."); + } + + if (!(unnest.unnestExpr instanceof RexInputRef)) { + // could be supported; but is there a need? + return; + } + if (input.cardinality() != 1) { + return; + } + + int inputIndex = input.nextSetBit(0); + + + List<RexNode> projects = new ArrayList<>(project.getProjects()); + RexNode unnestInput = projects.get(inputIndex); + + + projects.set( + inputIndex, + call.builder().getRexBuilder().makeInputRef(project.getInput(), 0) + ); + + RexNode newUnnestExpr = unnestInput.accept(new ExpressionPullerRexShuttle(projects, inputIndex)); + + if (projects.size() != project.getProjects().size()) { + // lets leave this for later + return; + } + + + RelNode newInputRel = call.builder() + .push(project.getInput()) + .project(projects) + .build(); + + + RelNode newUnnest = new LogicalUnnest( + unnest.getCluster(), unnest.getTraitSet(), newInputRel, newUnnestExpr, + unnest.getRowType(), unnest.filter + ); + call.transformTo(newUnnest); + call.getPlanner().prune(unnest); + } + + /** + * Pulls an expression thru a {@link Project}. + * + * May add new projections to the passed mutable list. + */ + private static class ExpressionPullerRexShuttle extends RexShuttle + { + private final List<RexNode> projects; + private int replaceableIndex; + + private ExpressionPullerRexShuttle(List<RexNode> projects, int replaceableIndex) + { + this.projects = projects; + this.replaceableIndex = replaceableIndex; + } + + public RexNode visitInputRef(RexInputRef inputRef) Review Comment: ## Missing Override annotation This method overrides [RexShuttle.visitInputRef](1); it is advisable to add an Override annotation. [Show more details](https://github.com/apache/druid/security/code-scanning/8351) ########## sql/src/main/java/org/apache/druid/sql/calcite/rule/logical/DruidUnnestRule.java: ########## @@ -0,0 +1,57 @@ +/* + * 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.druid.sql.calcite.rule.logical; + +import org.apache.calcite.plan.RelTrait; +import org.apache.calcite.plan.RelTraitSet; +import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.convert.ConverterRule; +import org.apache.druid.sql.calcite.rel.logical.DruidLogicalConvention; + +public class DruidUnnestRule extends ConverterRule +{ + + public DruidUnnestRule( + Class<? extends RelNode> clazz, + RelTrait in, + RelTrait out, + String descriptionPrefix) + { + super(clazz, in, out, descriptionPrefix); Review Comment: ## Deprecated method or constructor invocation Invoking [ConverterRule.ConverterRule](1) should be avoided because it has been deprecated. [Show more details](https://github.com/apache/druid/security/code-scanning/8353) ########## sql/src/main/java/org/apache/druid/sql/calcite/rule/logical/UnnestInputCleanupRule.java: ########## @@ -0,0 +1,144 @@ +/* + * 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.druid.sql.calcite.rule.logical; + +import org.apache.calcite.plan.RelOptRule; +import org.apache.calcite.plan.RelOptRuleCall; +import org.apache.calcite.plan.RelOptUtil.InputFinder; +import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.core.Project; +import org.apache.calcite.rel.rules.SubstitutionRule; +import org.apache.calcite.rex.RexInputRef; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.rex.RexShuttle; +import org.apache.calcite.util.ImmutableBitSet; +import org.apache.druid.error.DruidException; +import java.util.ArrayList; +import java.util.List; + +/** + * Makes tweaks to LogicalUnnest input. + * + * Removes any MV_TO_ARRAY call if its present for the input of the + * {@link LogicalUnnest}. + * + */ +public class UnnestInputCleanupRule extends RelOptRule implements SubstitutionRule +{ + public UnnestInputCleanupRule() + { + super( + operand( + LogicalUnnest.class, + operand(Project.class, any()) Review Comment: ## Deprecated method or constructor invocation Invoking [RelOptRule.operand](1) should be avoided because it has been deprecated. [Show more details](https://github.com/apache/druid/security/code-scanning/8357) ########## sql/src/main/java/org/apache/druid/sql/calcite/rule/logical/UnnestInputCleanupRule.java: ########## @@ -0,0 +1,144 @@ +/* + * 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.druid.sql.calcite.rule.logical; + +import org.apache.calcite.plan.RelOptRule; +import org.apache.calcite.plan.RelOptRuleCall; +import org.apache.calcite.plan.RelOptUtil.InputFinder; +import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.core.Project; +import org.apache.calcite.rel.rules.SubstitutionRule; +import org.apache.calcite.rex.RexInputRef; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.rex.RexShuttle; +import org.apache.calcite.util.ImmutableBitSet; +import org.apache.druid.error.DruidException; +import java.util.ArrayList; +import java.util.List; + +/** + * Makes tweaks to LogicalUnnest input. + * + * Removes any MV_TO_ARRAY call if its present for the input of the + * {@link LogicalUnnest}. + * + */ +public class UnnestInputCleanupRule extends RelOptRule implements SubstitutionRule +{ + public UnnestInputCleanupRule() + { + super( + operand( + LogicalUnnest.class, + operand(Project.class, any()) + ) Review Comment: ## Deprecated method or constructor invocation Invoking [RelOptRule.operand](1) should be avoided because it has been deprecated. [Show more details](https://github.com/apache/druid/security/code-scanning/8356) ########## sql/src/main/java/org/apache/druid/sql/calcite/rule/logical/UnnestInputCleanupRule.java: ########## @@ -0,0 +1,144 @@ +/* + * 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.druid.sql.calcite.rule.logical; + +import org.apache.calcite.plan.RelOptRule; +import org.apache.calcite.plan.RelOptRuleCall; +import org.apache.calcite.plan.RelOptUtil.InputFinder; +import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.core.Project; +import org.apache.calcite.rel.rules.SubstitutionRule; +import org.apache.calcite.rex.RexInputRef; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.rex.RexShuttle; +import org.apache.calcite.util.ImmutableBitSet; +import org.apache.druid.error.DruidException; +import java.util.ArrayList; +import java.util.List; + +/** + * Makes tweaks to LogicalUnnest input. + * + * Removes any MV_TO_ARRAY call if its present for the input of the + * {@link LogicalUnnest}. + * + */ +public class UnnestInputCleanupRule extends RelOptRule implements SubstitutionRule +{ + public UnnestInputCleanupRule() + { + super( + operand( + LogicalUnnest.class, + operand(Project.class, any()) Review Comment: ## Deprecated method or constructor invocation Invoking [RelOptRule.any](1) should be avoided because it has been deprecated. [Show more details](https://github.com/apache/druid/security/code-scanning/8358) ########## sql/src/main/java/org/apache/druid/sql/calcite/rule/logical/LogicalUnnestRule.java: ########## @@ -0,0 +1,160 @@ +/* + * 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.druid.sql.calcite.rule.logical; + +import com.google.common.collect.Iterables; +import org.apache.calcite.plan.RelOptRule; +import org.apache.calcite.plan.RelOptRuleCall; +import org.apache.calcite.plan.RelOptUtil; +import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.core.Filter; +import org.apache.calcite.rel.core.Project; +import org.apache.calcite.rel.core.Uncollect; +import org.apache.calcite.rel.logical.LogicalCorrelate; +import org.apache.calcite.rel.logical.LogicalValues; +import org.apache.calcite.rel.rules.SubstitutionRule; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.tools.RelBuilder; +import org.apache.druid.error.DruidException; +import org.apache.druid.sql.calcite.rel.DruidCorrelateUnnestRel; + +/** + * Recognizes a LogicalUnnest operation in the plan. + * + * Matches on the layout: + * + * <pre> + * LogicalCorrelate(correlation=[$cor0], joinType=[inner], requiredColumns=[{4}]) + * RelNodeSubtree + * Uncollect + * LogicalProject(arrayLongNulls=[$cor0.arrayLongNulls]) + * LogicalValues(tuples=[[{ 0 }]]) + * </pre> + * + * Translates it to use a {@link LogicalUnnest} like: + * + * <pre> + * LogicalUnnest(unnestExpr=[$cor0.arrayLongNulls]) + * RelNodeSubtree + * </pre> + * + * It raises an error for cases when {@link LogicalCorrelate} can't be + * translated as those are currently unsupported in Druid. + */ +public class LogicalUnnestRule extends RelOptRule implements SubstitutionRule +{ + public LogicalUnnestRule() + { + super(operand(LogicalCorrelate.class, any())); Review Comment: ## Deprecated method or constructor invocation Invoking [RelOptRule.any](1) should be avoided because it has been deprecated. [Show more details](https://github.com/apache/druid/security/code-scanning/8355) ########## sql/src/test/java/org/apache/druid/sql/calcite/DecoupledExtension.java: ########## @@ -101,6 +105,16 @@ }; QueryTestBuilder builder = new QueryTestBuilder(testConfig) + { + public QueryTestBuilder expectedQueries(List<Query<?>> expectedQueries) Review Comment: ## Missing Override annotation This method overrides [QueryTestBuilder.expectedQueries](1); it is advisable to add an Override annotation. [Show more details](https://github.com/apache/druid/security/code-scanning/8352) -- 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]
