github-code-scanning[bot] commented on code in PR #14510:
URL: https://github.com/apache/druid/pull/14510#discussion_r1289224758
##########
sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidCorrelateUnnestRel.java:
##########
@@ -184,6 +194,105 @@
unnestFilterOnDataSource = null;
}
+ final DruidRel<?> newLeftDruidRel;
+ final DruidQuery updatedLeftQuery;
+ // For some queries, with Calcite 1.35
+ // The plan has an MV_TO_ARRAY on the left side
+ // with a reference to it on the right side
+ // IN such a case we use a RexShuttle to remove the reference on the left
+ // And rewrite the left project
+ if (unnestDatasourceRel.getInputRexNode().getKind() ==
SqlKind.FIELD_ACCESS) {
+ final PartialDruidQuery leftPartialQueryToBeUpdated;
+ if (leftDruidRel instanceof DruidOuterQueryRel) {
+ leftPartialQueryToBeUpdated = ((DruidRel)
leftDruidRel.getInputs().get(0)).getPartialDruidQuery();
+ } else {
+ leftPartialQueryToBeUpdated = leftPartialQuery;
+ }
+ final Project leftProject =
leftPartialQueryToBeUpdated.getSelectProject();
+ final String dimensionToUpdate = expressionToUnnest.getDirectColumn();
+ final LogicalProject newProject;
+ if (leftProject == null) {
+ newProject = null;
+ } else {
+ // Use shuttle to update left project
+ final ProjectUpdateShuttle pus = new ProjectUpdateShuttle(
+ unwrapMvToArray(rexNodeToUnnest),
+ leftProject,
+ dimensionToUpdate
+ );
+ final List<RexNode> out = pus.visitList(leftProject.getProjects());
+ final RelDataType structType =
RexUtil.createStructType(getCluster().getTypeFactory(), out,
pus.getTypeNames());
+ newProject = LogicalProject.create(
+ leftProject.getInput(),
+ leftProject.getHints(),
+ out,
+ structType
+ );
+ }
+
+ final DruidRel leftFinalRel;
+ if (leftDruidRel instanceof DruidOuterQueryRel) {
+ leftFinalRel = (DruidRel) leftDruidRel.getInputs().get(0);
+ } else {
+ leftFinalRel = leftDruidRel;
+ }
+ final PartialDruidQuery pq =
PartialDruidQuery.create(leftPartialQueryToBeUpdated.getScan())
+
.withWhereFilter(leftPartialQueryToBeUpdated.getWhereFilter())
+
.withSelectProject(newProject)
+
.withSort(leftPartialQueryToBeUpdated.getSort());
+
+ // The same MV_TO_ARRAY on left and reference to the right can happen
during
+ // SORT_PROJECT and SELECT_PROJECT stages
+ // We use the same methodology of using a shuttle to rewrite the
projects.
+ if (leftPartialQuery.stage() == PartialDruidQuery.Stage.SORT_PROJECT) {
+ final Project sortProject =
leftPartialQueryToBeUpdated.getSortProject();
+ final ProjectUpdateShuttle pus = new ProjectUpdateShuttle(
+ unwrapMvToArray(rexNodeToUnnest),
+ sortProject,
+ dimensionToUpdate
+ );
+ final List<RexNode> out = pus.visitList(sortProject.getProjects());
+ final RelDataType structType =
RexUtil.createStructType(getCluster().getTypeFactory(), out,
pus.getTypeNames());
+ final Project newSortProject = LogicalProject.create(
+ sortProject.getInput(),
+ sortProject.getHints(),
+ out,
+ structType
+ );
Review Comment:
## Deprecated method or constructor invocation
Invoking [LogicalProject.create](1) should be avoided because it has been
deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/5473)
##########
sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidCorrelateUnnestRel.java:
##########
@@ -184,6 +194,105 @@
unnestFilterOnDataSource = null;
}
+ final DruidRel<?> newLeftDruidRel;
+ final DruidQuery updatedLeftQuery;
+ // For some queries, with Calcite 1.35
+ // The plan has an MV_TO_ARRAY on the left side
+ // with a reference to it on the right side
+ // IN such a case we use a RexShuttle to remove the reference on the left
+ // And rewrite the left project
+ if (unnestDatasourceRel.getInputRexNode().getKind() ==
SqlKind.FIELD_ACCESS) {
+ final PartialDruidQuery leftPartialQueryToBeUpdated;
+ if (leftDruidRel instanceof DruidOuterQueryRel) {
+ leftPartialQueryToBeUpdated = ((DruidRel)
leftDruidRel.getInputs().get(0)).getPartialDruidQuery();
+ } else {
+ leftPartialQueryToBeUpdated = leftPartialQuery;
+ }
+ final Project leftProject =
leftPartialQueryToBeUpdated.getSelectProject();
+ final String dimensionToUpdate = expressionToUnnest.getDirectColumn();
+ final LogicalProject newProject;
+ if (leftProject == null) {
+ newProject = null;
+ } else {
+ // Use shuttle to update left project
+ final ProjectUpdateShuttle pus = new ProjectUpdateShuttle(
+ unwrapMvToArray(rexNodeToUnnest),
+ leftProject,
+ dimensionToUpdate
+ );
+ final List<RexNode> out = pus.visitList(leftProject.getProjects());
+ final RelDataType structType =
RexUtil.createStructType(getCluster().getTypeFactory(), out,
pus.getTypeNames());
Review Comment:
## Deprecated method or constructor invocation
Invoking [RexUtil.createStructType](1) should be avoided because it has been
deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/5470)
##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/CalcitePlanner.java:
##########
@@ -184,20 +189,15 @@
reset();
break;
default:
+ break;
}
- ensure(State.STATE_1_RESET);
+ ensure(CalcitePlanner.State.STATE_1_RESET);
- RelDataTypeSystem typeSystem =
- connectionConfig.typeSystem(RelDataTypeSystem.class,
- RelDataTypeSystem.DEFAULT);
typeFactory = new JavaTypeFactoryImpl(typeSystem);
- planner = new VolcanoPlanner(frameworkConfig.getCostFactory(), context);
- RelOptUtil.registerDefaultRules(planner,
- connectionConfig.materializationsEnabled(),
- Hook.ENABLE_BINDABLE.get(false));
+ RelOptPlanner planner = this.planner = new VolcanoPlanner(costFactory,
context);
Review Comment:
## Possible confusion of local and field
Potentially confusing name: method [ready](1) also refers to field
[planner](2) (as this.planner).
[Show more
details](https://github.com/apache/druid/security/code-scanning/5584)
##########
sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidCorrelateUnnestRel.java:
##########
@@ -184,6 +194,105 @@
unnestFilterOnDataSource = null;
}
+ final DruidRel<?> newLeftDruidRel;
+ final DruidQuery updatedLeftQuery;
+ // For some queries, with Calcite 1.35
+ // The plan has an MV_TO_ARRAY on the left side
+ // with a reference to it on the right side
+ // IN such a case we use a RexShuttle to remove the reference on the left
+ // And rewrite the left project
+ if (unnestDatasourceRel.getInputRexNode().getKind() ==
SqlKind.FIELD_ACCESS) {
+ final PartialDruidQuery leftPartialQueryToBeUpdated;
+ if (leftDruidRel instanceof DruidOuterQueryRel) {
+ leftPartialQueryToBeUpdated = ((DruidRel)
leftDruidRel.getInputs().get(0)).getPartialDruidQuery();
+ } else {
+ leftPartialQueryToBeUpdated = leftPartialQuery;
+ }
+ final Project leftProject =
leftPartialQueryToBeUpdated.getSelectProject();
+ final String dimensionToUpdate = expressionToUnnest.getDirectColumn();
+ final LogicalProject newProject;
+ if (leftProject == null) {
+ newProject = null;
+ } else {
+ // Use shuttle to update left project
+ final ProjectUpdateShuttle pus = new ProjectUpdateShuttle(
+ unwrapMvToArray(rexNodeToUnnest),
+ leftProject,
+ dimensionToUpdate
+ );
+ final List<RexNode> out = pus.visitList(leftProject.getProjects());
+ final RelDataType structType =
RexUtil.createStructType(getCluster().getTypeFactory(), out,
pus.getTypeNames());
+ newProject = LogicalProject.create(
+ leftProject.getInput(),
+ leftProject.getHints(),
+ out,
+ structType
+ );
+ }
+
+ final DruidRel leftFinalRel;
+ if (leftDruidRel instanceof DruidOuterQueryRel) {
+ leftFinalRel = (DruidRel) leftDruidRel.getInputs().get(0);
+ } else {
+ leftFinalRel = leftDruidRel;
+ }
+ final PartialDruidQuery pq =
PartialDruidQuery.create(leftPartialQueryToBeUpdated.getScan())
+
.withWhereFilter(leftPartialQueryToBeUpdated.getWhereFilter())
+
.withSelectProject(newProject)
+
.withSort(leftPartialQueryToBeUpdated.getSort());
+
+ // The same MV_TO_ARRAY on left and reference to the right can happen
during
+ // SORT_PROJECT and SELECT_PROJECT stages
+ // We use the same methodology of using a shuttle to rewrite the
projects.
+ if (leftPartialQuery.stage() == PartialDruidQuery.Stage.SORT_PROJECT) {
+ final Project sortProject =
leftPartialQueryToBeUpdated.getSortProject();
+ final ProjectUpdateShuttle pus = new ProjectUpdateShuttle(
+ unwrapMvToArray(rexNodeToUnnest),
+ sortProject,
+ dimensionToUpdate
+ );
+ final List<RexNode> out = pus.visitList(sortProject.getProjects());
+ final RelDataType structType =
RexUtil.createStructType(getCluster().getTypeFactory(), out,
pus.getTypeNames());
Review Comment:
## Deprecated method or constructor invocation
Invoking [RexUtil.createStructType](1) should be avoided because it has been
deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/5472)
##########
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchObjectSqlAggregator.java:
##########
@@ -36,15 +36,15 @@
{
private static final String NAME = "DS_HLL";
private static final SqlAggFunction FUNCTION_INSTANCE =
- OperatorConversions.aggregatorBuilder(NAME)
- .operandNames("column", "lgK", "tgtHllType")
- .operandTypes(SqlTypeFamily.ANY,
SqlTypeFamily.NUMERIC, SqlTypeFamily.STRING)
- .operandTypeInference(InferTypes.VARCHAR_1024)
- .requiredOperandCount(1)
- .literalOperands(1, 2)
- .returnTypeNonNull(SqlTypeName.OTHER)
-
.functionCategory(SqlFunctionCategory.USER_DEFINED_FUNCTION)
- .build();
+ (SqlAggFunction) OperatorConversions.aggregatorBuilder(NAME)
+ .operandNames("column", "lgK", "tgtHllType")
+ .operandTypes(SqlTypeFamily.ANY,
SqlTypeFamily.NUMERIC, SqlTypeFamily.STRING)
+ .operandTypeInference(InferTypes.VARCHAR_1024)
+ .requiredOperandCount(1)
Review Comment:
## Deprecated method or constructor invocation
Invoking [OperatorBuilder.requiredOperandCount](1) should be avoided because
it has been deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/5435)
##########
sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidCorrelateUnnestRel.java:
##########
@@ -184,6 +194,105 @@
unnestFilterOnDataSource = null;
}
+ final DruidRel<?> newLeftDruidRel;
+ final DruidQuery updatedLeftQuery;
+ // For some queries, with Calcite 1.35
+ // The plan has an MV_TO_ARRAY on the left side
+ // with a reference to it on the right side
+ // IN such a case we use a RexShuttle to remove the reference on the left
+ // And rewrite the left project
+ if (unnestDatasourceRel.getInputRexNode().getKind() ==
SqlKind.FIELD_ACCESS) {
+ final PartialDruidQuery leftPartialQueryToBeUpdated;
+ if (leftDruidRel instanceof DruidOuterQueryRel) {
+ leftPartialQueryToBeUpdated = ((DruidRel)
leftDruidRel.getInputs().get(0)).getPartialDruidQuery();
+ } else {
+ leftPartialQueryToBeUpdated = leftPartialQuery;
+ }
+ final Project leftProject =
leftPartialQueryToBeUpdated.getSelectProject();
+ final String dimensionToUpdate = expressionToUnnest.getDirectColumn();
+ final LogicalProject newProject;
+ if (leftProject == null) {
+ newProject = null;
+ } else {
+ // Use shuttle to update left project
+ final ProjectUpdateShuttle pus = new ProjectUpdateShuttle(
+ unwrapMvToArray(rexNodeToUnnest),
+ leftProject,
+ dimensionToUpdate
+ );
+ final List<RexNode> out = pus.visitList(leftProject.getProjects());
+ final RelDataType structType =
RexUtil.createStructType(getCluster().getTypeFactory(), out,
pus.getTypeNames());
+ newProject = LogicalProject.create(
+ leftProject.getInput(),
+ leftProject.getHints(),
+ out,
+ structType
+ );
Review Comment:
## Deprecated method or constructor invocation
Invoking [LogicalProject.create](1) should be avoided because it has been
deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/5471)
##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/CalcitePlanner.java:
##########
@@ -279,37 +282,72 @@
@Override
public RelRoot rel(SqlNode sql)
{
- ensure(State.STATE_4_VALIDATED);
- assert validatedSqlNode != null;
+ ensure(CalcitePlanner.State.STATE_4_VALIDATED);
+ SqlNode validatedSqlNode = Objects.requireNonNull(
+ this.validatedSqlNode,
+ "validatedSqlNode is null. Need to call #validate() first"
+ );
Review Comment:
## Possible confusion of local and field
Potentially confusing name: method [rel](1) also refers to field
[validatedSqlNode](2) (as this.validatedSqlNode).
[Show more
details](https://github.com/apache/druid/security/code-scanning/5585)
##########
processing/src/main/java/org/apache/druid/segment/FilteredStorageAdapter.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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.segment;
+
+import com.google.common.collect.ImmutableList;
+import org.apache.druid.java.util.common.granularity.Granularity;
+import org.apache.druid.java.util.common.guava.Sequence;
+import org.apache.druid.query.QueryMetrics;
+import org.apache.druid.query.filter.DimFilter;
+import org.apache.druid.query.filter.Filter;
+import org.apache.druid.segment.column.ColumnCapabilities;
+import org.apache.druid.segment.data.Indexed;
+import org.apache.druid.segment.filter.AndFilter;
+import org.joda.time.DateTime;
+import org.joda.time.Interval;
+
+import javax.annotation.Nullable;
+
+public class FilteredStorageAdapter implements StorageAdapter
+{
+ private final DimFilter filterOnDataSource;
+ private final StorageAdapter baseStorageAdapter;
+
+ public FilteredStorageAdapter(final StorageAdapter adapter, final DimFilter
filter)
+ {
+ this.baseStorageAdapter = adapter;
+ this.filterOnDataSource = filter;
+ }
+
+ @Override
+ public Sequence<Cursor> makeCursors(
+ @Nullable Filter filter,
+ Interval interval,
+ VirtualColumns virtualColumns,
+ Granularity gran,
+ boolean descending,
+ @Nullable QueryMetrics<?> queryMetrics
+ )
+ {
+ final Filter andFilter;
+ if (filter == null) {
+ if (filterOnDataSource != null) {
+ andFilter = filterOnDataSource.toFilter();
+ } else {
+ andFilter = null;
+ }
+ } else {
+ andFilter = new AndFilter(ImmutableList.of(filter,
filterOnDataSource.toFilter()));
Review Comment:
## Dereferenced variable may be null
Variable [filterOnDataSource](1) may be null at this access as suggested by
[this](2) null guard.
[Show more
details](https://github.com/apache/druid/security/code-scanning/5650)
##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/CalcitePlanner.java:
##########
@@ -353,71 +384,103 @@
// CalciteCatalogReader is stateless; no need to store one
private CalciteCatalogReader createCatalogReader()
{
+ SchemaPlus defaultSchema = Objects.requireNonNull(this.defaultSchema,
"defaultSchema");
Review Comment:
## Possible confusion of local and field
Potentially confusing name: method [createCatalogReader](1) also refers to
field [defaultSchema](2) (as this.defaultSchema).
[Show more
details](https://github.com/apache/druid/security/code-scanning/5586)
--
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]
