You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by so...@apache.org on 2024/01/12 04:36:29 UTC

(druid) branch master updated: Remove UnaryFunctionOperatorConversion and RoundOperatorConversion (#15566)

This is an automated email from the ASF dual-hosted git repository.

soumyava pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git


The following commit(s) were added to refs/heads/master by this push:
     new e597cc2949c Remove UnaryFunctionOperatorConversion and RoundOperatorConversion (#15566)
e597cc2949c is described below

commit e597cc2949c9ffe6a18ec2efc8ee9880e029d44c
Author: Zoltan Haindrich <ki...@rxd.hu>
AuthorDate: Fri Jan 12 05:36:23 2024 +0100

    Remove UnaryFunctionOperatorConversion and RoundOperatorConversion (#15566)
    
    * get rid of roun op conv
    
    * cleanup
    
    * use DirectOperatorConversion instead unary
    
    * import order
---
 .../UnaryFunctionOperatorConversion.java           | 64 ----------------------
 .../builtin/RoundOperatorConversion.java           | 39 -------------
 .../sql/calcite/planner/DruidOperatorTable.java    | 11 ++--
 .../sql/calcite/expression/ExpressionsTest.java    | 21 +++++--
 4 files changed, 21 insertions(+), 114 deletions(-)

diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/UnaryFunctionOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/UnaryFunctionOperatorConversion.java
deleted file mode 100644
index 072ae28450f..00000000000
--- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/UnaryFunctionOperatorConversion.java
+++ /dev/null
@@ -1,64 +0,0 @@
-/*
- * 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.expression;
-
-import com.google.common.collect.Iterables;
-import org.apache.calcite.rex.RexNode;
-import org.apache.calcite.sql.SqlOperator;
-import org.apache.druid.java.util.common.StringUtils;
-import org.apache.druid.segment.column.RowSignature;
-import org.apache.druid.sql.calcite.planner.PlannerContext;
-
-public class UnaryFunctionOperatorConversion implements SqlOperatorConversion
-{
-  private final SqlOperator operator;
-  private final String druidOperator;
-
-  public UnaryFunctionOperatorConversion(final SqlOperator operator, final String druidOperator)
-  {
-    this.operator = operator;
-    this.druidOperator = druidOperator;
-  }
-
-  @Override
-  public SqlOperator calciteOperator()
-  {
-    return operator;
-  }
-
-  @Override
-  public DruidExpression toDruidExpression(
-      final PlannerContext plannerContext,
-      final RowSignature rowSignature,
-      final RexNode rexNode
-  )
-  {
-    return OperatorConversions.convertCallBuilder(
-        plannerContext,
-        rowSignature,
-        rexNode,
-        operands -> StringUtils.format(
-            "%s(%s)",
-            druidOperator,
-            Iterables.getOnlyElement(operands).getExpression()
-        )
-    );
-  }
-}
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/RoundOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/RoundOperatorConversion.java
deleted file mode 100644
index e8031e55aee..00000000000
--- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/RoundOperatorConversion.java
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * 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.expression.builtin;
-
-import org.apache.calcite.sql.SqlFunction;
-import org.apache.calcite.sql.fun.SqlStdOperatorTable;
-import org.apache.druid.math.expr.Function;
-import org.apache.druid.sql.calcite.expression.DirectOperatorConversion;
-
-public class RoundOperatorConversion extends DirectOperatorConversion
-{
-  public RoundOperatorConversion()
-  {
-    super(SqlStdOperatorTable.ROUND, Function.Round.NAME);
-  }
-
-  @Override
-  public SqlFunction calciteOperator()
-  {
-    return SqlStdOperatorTable.ROUND;
-  }
-}
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidOperatorTable.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidOperatorTable.java
index 3bcb6f1b177..f877a107210 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidOperatorTable.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidOperatorTable.java
@@ -54,7 +54,6 @@ import org.apache.druid.sql.calcite.expression.BinaryOperatorConversion;
 import org.apache.druid.sql.calcite.expression.DirectOperatorConversion;
 import org.apache.druid.sql.calcite.expression.OperatorConversions;
 import org.apache.druid.sql.calcite.expression.SqlOperatorConversion;
-import org.apache.druid.sql.calcite.expression.UnaryFunctionOperatorConversion;
 import org.apache.druid.sql.calcite.expression.UnaryPrefixOperatorConversion;
 import org.apache.druid.sql.calcite.expression.WindowSqlAggregate;
 import org.apache.druid.sql.calcite.expression.builtin.ArrayAppendOperatorConversion;
@@ -109,7 +108,6 @@ import org.apache.druid.sql.calcite.expression.builtin.ReinterpretOperatorConver
 import org.apache.druid.sql.calcite.expression.builtin.RepeatOperatorConversion;
 import org.apache.druid.sql.calcite.expression.builtin.ReverseOperatorConversion;
 import org.apache.druid.sql.calcite.expression.builtin.RightOperatorConversion;
-import org.apache.druid.sql.calcite.expression.builtin.RoundOperatorConversion;
 import org.apache.druid.sql.calcite.expression.builtin.SafeDivideOperatorConversion;
 import org.apache.druid.sql.calcite.expression.builtin.SearchOperatorConversion;
 import org.apache.druid.sql.calcite.expression.builtin.StringFormatOperatorConversion;
@@ -130,6 +128,7 @@ import org.apache.druid.sql.calcite.expression.builtin.TruncateOperatorConversio
 import org.apache.druid.sql.calcite.planner.convertlet.DruidConvertletTable;
 
 import javax.annotation.Nullable;
+
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -354,6 +353,8 @@ public class DruidOperatorTable implements SqlOperatorTable
                    .add(new NestedDataOperatorConversions.TryParseJsonOperatorConversion())
                    .build();
 
+  public static final DirectOperatorConversion ROUND_OPERATOR_CONVERSION = new DirectOperatorConversion(SqlStdOperatorTable.ROUND, "round");
+
   private static final List<SqlOperatorConversion> STANDARD_OPERATOR_CONVERSIONS =
       ImmutableList.<SqlOperatorConversion>builder()
                    .add(new DirectOperatorConversion(SqlStdOperatorTable.ABS, "abs"))
@@ -385,8 +386,8 @@ public class DruidOperatorTable implements SqlOperatorTable
                    .add(new DirectOperatorConversion(SqlStdOperatorTable.DEGREES, "toDegrees"))
                    .add(new UnaryPrefixOperatorConversion(SqlStdOperatorTable.NOT, "!"))
                    .add(new UnaryPrefixOperatorConversion(SqlStdOperatorTable.UNARY_MINUS, "-"))
-                   .add(new UnaryFunctionOperatorConversion(SqlStdOperatorTable.IS_NULL, "isnull"))
-                   .add(new UnaryFunctionOperatorConversion(SqlStdOperatorTable.IS_NOT_NULL, "notnull"))
+                   .add(new DirectOperatorConversion(SqlStdOperatorTable.IS_NULL, "isnull"))
+                   .add(new DirectOperatorConversion(SqlStdOperatorTable.IS_NOT_NULL, "notnull"))
                    .add(new DirectOperatorConversion(SqlStdOperatorTable.IS_DISTINCT_FROM, "isdistinctfrom"))
                    .add(new DirectOperatorConversion(SqlStdOperatorTable.IS_NOT_DISTINCT_FROM, "notdistinctfrom"))
                    .add(new DirectOperatorConversion(SqlStdOperatorTable.IS_FALSE, "isfalse"))
@@ -410,7 +411,7 @@ public class DruidOperatorTable implements SqlOperatorTable
                    .add(new BinaryOperatorConversion(SqlStdOperatorTable.AND, "&&"))
                    .add(new BinaryOperatorConversion(SqlStdOperatorTable.OR, "||"))
                    .add(new SearchOperatorConversion())
-                   .add(new RoundOperatorConversion())
+                   .add(ROUND_OPERATOR_CONVERSION)
                    .addAll(TIME_OPERATOR_CONVERSIONS)
                    .addAll(STRING_OPERATOR_CONVERSIONS)
                    .addAll(VALUE_COERCION_OPERATOR_CONVERSIONS)
diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java
index 0273e7d2aa9..8656f1221f2 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java
@@ -25,6 +25,7 @@ import org.apache.calcite.avatica.util.TimeUnit;
 import org.apache.calcite.avatica.util.TimeUnitRange;
 import org.apache.calcite.sql.SqlFunction;
 import org.apache.calcite.sql.SqlIntervalQualifier;
+import org.apache.calcite.sql.SqlOperator;
 import org.apache.calcite.sql.fun.SqlStdOperatorTable;
 import org.apache.calcite.sql.fun.SqlTrimFunction;
 import org.apache.calcite.sql.parser.SqlParserPos;
@@ -54,7 +55,6 @@ import org.apache.druid.sql.calcite.expression.builtin.RegexpReplaceOperatorConv
 import org.apache.druid.sql.calcite.expression.builtin.RepeatOperatorConversion;
 import org.apache.druid.sql.calcite.expression.builtin.ReverseOperatorConversion;
 import org.apache.druid.sql.calcite.expression.builtin.RightOperatorConversion;
-import org.apache.druid.sql.calcite.expression.builtin.RoundOperatorConversion;
 import org.apache.druid.sql.calcite.expression.builtin.StringFormatOperatorConversion;
 import org.apache.druid.sql.calcite.expression.builtin.StrposOperatorConversion;
 import org.apache.druid.sql.calcite.expression.builtin.SubstringOperatorConversion;
@@ -65,6 +65,7 @@ import org.apache.druid.sql.calcite.expression.builtin.TimeFormatOperatorConvers
 import org.apache.druid.sql.calcite.expression.builtin.TimeParseOperatorConversion;
 import org.apache.druid.sql.calcite.expression.builtin.TimeShiftOperatorConversion;
 import org.apache.druid.sql.calcite.expression.builtin.TruncateOperatorConversion;
+import org.apache.druid.sql.calcite.planner.DruidOperatorTable;
 import org.apache.druid.sql.calcite.util.CalciteTestBase;
 import org.joda.time.Period;
 import org.junit.Assert;
@@ -129,12 +130,19 @@ public class ExpressionsTest extends CalciteTestBase
 
   private ExpressionTestHelper testHelper;
 
+  final DruidOperatorTable operatorTable = new DruidOperatorTable(Collections.emptySet(), Collections.emptySet());
+
   @Before
   public void setUp()
   {
     testHelper = new ExpressionTestHelper(ROW_SIGNATURE, BINDINGS);
   }
 
+  private SqlOperatorConversion getOperatorConversion(SqlFunction round)
+  {
+    return operatorTable.lookupOperatorConversion(round);
+  }
+
   @Test
   public void testConcat()
   {
@@ -1151,7 +1159,7 @@ public class ExpressionsTest extends CalciteTestBase
   @Test
   public void testRound()
   {
-    final SqlFunction roundFunction = new RoundOperatorConversion().calciteOperator();
+    final SqlOperator roundFunction = getOperatorConversion(SqlStdOperatorTable.ROUND).calciteOperator();
 
     testHelper.testExpression(
         roundFunction,
@@ -1256,7 +1264,8 @@ public class ExpressionsTest extends CalciteTestBase
   @Test
   public void testRoundWithInvalidArgument()
   {
-    final SqlFunction roundFunction = new RoundOperatorConversion().calciteOperator();
+
+    final SqlOperator roundFunction = getOperatorConversion(SqlStdOperatorTable.ROUND).calciteOperator();
 
     if (!NullHandling.sqlCompatible()) {
       Throwable t = Assert.assertThrows(
@@ -1284,7 +1293,7 @@ public class ExpressionsTest extends CalciteTestBase
   @Test
   public void testRoundWithInvalidSecondArgument()
   {
-    final SqlFunction roundFunction = new RoundOperatorConversion().calciteOperator();
+    final SqlOperator roundFunction = getOperatorConversion(SqlStdOperatorTable.ROUND).calciteOperator();
 
     Throwable t = Assert.assertThrows(
         DruidException.class,
@@ -1311,7 +1320,7 @@ public class ExpressionsTest extends CalciteTestBase
   @Test
   public void testRoundWithNanShouldRoundTo0()
   {
-    final SqlFunction roundFunction = new RoundOperatorConversion().calciteOperator();
+    final SqlOperator roundFunction = getOperatorConversion(SqlStdOperatorTable.ROUND).calciteOperator();
 
     testHelper.testExpression(
         roundFunction,
@@ -1342,7 +1351,7 @@ public class ExpressionsTest extends CalciteTestBase
   @Test
   public void testRoundWithInfinityShouldRoundTo0()
   {
-    final SqlFunction roundFunction = new RoundOperatorConversion().calciteOperator();
+    final SqlOperator roundFunction = getOperatorConversion(SqlStdOperatorTable.ROUND).calciteOperator();
 
     //CHECKSTYLE.OFF: Regexp
     testHelper.testExpression(


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org