gianm commented on code in PR #18084:
URL: https://github.com/apache/druid/pull/18084#discussion_r2132091841


##########
sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/MultiValueStringOperatorConversions.java:
##########
@@ -403,14 +402,12 @@ public DruidExpression toDruidExpression(
           plannerContext,
           rowSignature,
           rexNode,
-          druidExpressions -> {
-            final List<DruidExpression> newArgs = 
harmonizeNullsMvdArg0OperandList(druidExpressions);
-            return DruidExpression.ofFunctionCall(
-                Calcites.getColumnTypeForRelDataType(rexNode.getType()),
-                getDruidFunctionName(),
-                newArgs
-            );
-          }
+          druidExpressions ->

Review Comment:
   Ah, I had deleted the other one, not this one you commented on. Just deleted 
this one too. Actually, I did a bigger change:
   
   - updated MV Contains and Overlap to call a superclass constructor with 
their operator and function names, rather than overriding `calciteOperator()` 
and `getDruidFunctionName()`
   - removed `toDruidExpression` and `toDruidExpressionWithPostAggOperands` 
from MV Contains and Overlap
   - marked `calciteOperator()` and `getDruidFunctionName()` as `final` in 
`DirectOperatorConversion`, so all the *other* methods on 
`DirectOperatorConversion` can be trusted to use the correct operator and 
function name



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