github-advanced-security[bot] commented on code in PR #15609:
URL: https://github.com/apache/druid/pull/15609#discussion_r1439392219


##########
sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/SearchOperatorConversion.java:
##########
@@ -83,7 +97,187 @@
     return Expressions.toDruidExpression(
         plannerContext,
         rowSignature,
-        RexUtil.expandSearch(REX_BUILDER, null, rexNode)
+        expandSearch((RexCall) rexNode, REX_BUILDER)
     );
   }
+
+  /**
+   * Like {@link RexUtil#expandSearch(RexBuilder, RexProgram, RexNode)}, but 
with some enhancements:
+   *
+   * 1) Expands NOT IN (a.k.a. {@link Sarg#isComplementedPoints()} as !(a || b 
|| c) rather than (!a && !b && !c),
+   * which helps us convert it to an {@link InDimFilter} later.
+   *
+   * 2) Can generate nice conversions for complement-points even if the range 
is not *entirely* complement-points.
+   */
+  public static RexNode expandSearch(
+      final RexCall call,
+      final RexBuilder rexBuilder
+  )
+  {
+    final RexNode arg = call.operands.get(0);
+    final RexLiteral sargRex = (RexLiteral) call.operands.get(1);
+    final Sarg<?> sarg = sargRex.getValueAs(Sarg.class);
+
+    if (sarg.isAll() || sarg.isNone()) {
+      return RexUtil.expandSearch(rexBuilder, null, call);
+    }
+
+    // RexNodes that represent ranges *including* the notInPoints. Later, the 
notInRexNode will be ANDed in so
+    // those notInPoints can be removed.
+    final List<RexNode> rangeRexNodes = new ArrayList<>();
+
+    // Compute points that occur in the complement of the range set. These are 
"NOT IN" points.
+    final List<Comparable> notInPoints;
+    final List<RexNode> notInRexNodes;

Review Comment:
   ## Unread local variable
   
   Variable 'List<RexNode> notInRexNodes' is never read.
   
   [Show more 
details](https://github.com/apache/druid/security/code-scanning/6388)



##########
sql/src/main/java/org/apache/druid/sql/calcite/rule/CaseToCoalesceRule.java:
##########
@@ -0,0 +1,125 @@
+/*
+ * 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;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.rules.SubstitutionRule;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexShuttle;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.validate.SqlValidator;
+import 
org.apache.druid.sql.calcite.aggregation.builtin.EarliestLatestAnySqlAggregator;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Rule that un-does the rewrite from {@link 
org.apache.calcite.sql.fun.SqlCoalesceFunction#rewriteCall}.
+ *
+ * Important because otherwise COALESCE turns into a gnarly CASE with 
duplicated expressions. We must un-do the
+ * rewrite rather than disable {@link SqlValidator.Config#callRewrite()}, 
because we rely on validator rewrites
+ * in other cases, such as {@link EarliestLatestAnySqlAggregator#EARLIEST} and
+ * {@link EarliestLatestAnySqlAggregator#LATEST}.
+ */
+public class CaseToCoalesceRule extends RelOptRule implements SubstitutionRule
+{
+  public CaseToCoalesceRule()
+  {
+    super(operand(RelNode.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/6390)



##########
sql/src/main/java/org/apache/druid/sql/calcite/rule/FilterDecomposeCoalesceRule.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * 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;
+
+import com.google.common.collect.ImmutableList;
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.rel.core.Filter;
+import org.apache.calcite.rel.rules.ReduceExpressionsRule;
+import org.apache.calcite.rel.rules.SubstitutionRule;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexShuttle;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.druid.sql.calcite.planner.Calcites;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Transform calls like f(COALESCE(x, y)) => (x IS NOT NULL AND f(x)) OR (x IS 
NULL AND f(y)), for boolean functions f.
+ *
+ * Only simple calls are transformed; see {@link #isSimpleCoalesce(RexNode)} 
for a definition. This is because the
+ * main purpose of this decomposition is to make it more likely that we'll be 
able to use indexes when filtering.
+ */
+public class FilterDecomposeCoalesceRule extends RelOptRule implements 
SubstitutionRule
+{
+  public FilterDecomposeCoalesceRule()
+  {
+    super(operand(Filter.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/6394)



##########
sql/src/main/java/org/apache/druid/sql/calcite/rule/CoalesceLookupRule.java:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.rules.SubstitutionRule;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexShuttle;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.SqlKind;
+import 
org.apache.druid.sql.calcite.expression.builtin.QueryLookupOperatorConversion;
+
+/**
+ * Rule that rewrites {@code COALESCE(LOOKUP(x, 'lookupName'), 
'missingValue')} to
+ * {@code LOOKUP(x, 'lookupName', 'missingValue')}.
+ */
+public class CoalesceLookupRule extends RelOptRule implements SubstitutionRule
+{
+  public CoalesceLookupRule()
+  {
+    super(operand(RelNode.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/6392)



##########
sql/src/main/java/org/apache/druid/sql/calcite/rule/CoalesceLookupRule.java:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.rules.SubstitutionRule;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexShuttle;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.SqlKind;
+import 
org.apache.druid.sql.calcite.expression.builtin.QueryLookupOperatorConversion;
+
+/**
+ * Rule that rewrites {@code COALESCE(LOOKUP(x, 'lookupName'), 
'missingValue')} to
+ * {@code LOOKUP(x, 'lookupName', 'missingValue')}.
+ */
+public class CoalesceLookupRule extends RelOptRule implements SubstitutionRule
+{
+  public CoalesceLookupRule()
+  {
+    super(operand(RelNode.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/6391)



##########
sql/src/main/java/org/apache/druid/sql/calcite/rule/CaseToCoalesceRule.java:
##########
@@ -0,0 +1,125 @@
+/*
+ * 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;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.rules.SubstitutionRule;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexShuttle;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.validate.SqlValidator;
+import 
org.apache.druid.sql.calcite.aggregation.builtin.EarliestLatestAnySqlAggregator;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Rule that un-does the rewrite from {@link 
org.apache.calcite.sql.fun.SqlCoalesceFunction#rewriteCall}.
+ *
+ * Important because otherwise COALESCE turns into a gnarly CASE with 
duplicated expressions. We must un-do the
+ * rewrite rather than disable {@link SqlValidator.Config#callRewrite()}, 
because we rely on validator rewrites
+ * in other cases, such as {@link EarliestLatestAnySqlAggregator#EARLIEST} and
+ * {@link EarliestLatestAnySqlAggregator#LATEST}.
+ */
+public class CaseToCoalesceRule extends RelOptRule implements SubstitutionRule
+{
+  public CaseToCoalesceRule()
+  {
+    super(operand(RelNode.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/6389)



##########
sql/src/main/java/org/apache/druid/sql/calcite/rule/FilterDecomposeCoalesceRule.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * 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;
+
+import com.google.common.collect.ImmutableList;
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.rel.core.Filter;
+import org.apache.calcite.rel.rules.ReduceExpressionsRule;
+import org.apache.calcite.rel.rules.SubstitutionRule;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexShuttle;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.druid.sql.calcite.planner.Calcites;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Transform calls like f(COALESCE(x, y)) => (x IS NOT NULL AND f(x)) OR (x IS 
NULL AND f(y)), for boolean functions f.
+ *
+ * Only simple calls are transformed; see {@link #isSimpleCoalesce(RexNode)} 
for a definition. This is because the
+ * main purpose of this decomposition is to make it more likely that we'll be 
able to use indexes when filtering.
+ */
+public class FilterDecomposeCoalesceRule extends RelOptRule implements 
SubstitutionRule
+{
+  public FilterDecomposeCoalesceRule()
+  {
+    super(operand(Filter.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/6393)



-- 
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]

Reply via email to