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


##########
sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java:
##########
@@ -306,61 +309,79 @@ private static DruidExpression rexCallToDruidExpression(
     }
   }
 
+  /**
+   * Create an {@link ExprEval} from a literal {@link RexNode}.
+   */
   @Nullable
-  static DruidExpression literalToDruidExpression(
+  public static ExprEvalWrapper literalToExprEval(
       final PlannerContext plannerContext,
       final RexNode rexNode
   )
   {
-    final SqlTypeName sqlTypeName = rexNode.getType().getSqlTypeName();
+    if (rexNode.isA(SqlKind.CAST)) {
+      if (SqlTypeFamily.DATE.contains(rexNode.getType())) {
+        // Cast to DATE suggests some timestamp flooring. We don't deal with 
that here, so return null.
+        return null;
+      }
+
+      final ExprEvalWrapper innerEval = literalToExprEval(plannerContext, 
((RexCall) rexNode).getOperands().get(0));
+      final ColumnType castToColumnType = 
Calcites.getColumnTypeForRelDataType(rexNode.getType());
+      if (castToColumnType == null) {
+        return null;
+      }
+
+      final ExpressionType castToExprType = 
ExpressionType.fromColumnType(castToColumnType);
+      if (castToExprType == null) {
+        return null;
+      }
+
+      return new ExprEvalWrapper(
+          innerEval.exprEval().castTo(castToExprType),
+          false
+      );
+    }
 
     // Translate literal.
-    final ColumnType columnType = 
Calcites.getColumnTypeForRelDataType(rexNode.getType());
+    final SqlTypeName sqlTypeName = rexNode.getType().getSqlTypeName();
+    final ExprEval<?> retVal;
+    boolean isEmptyString = false;
+
     if (RexLiteral.isNullLiteral(rexNode)) {
-      return DruidExpression.ofLiteral(columnType, 
DruidExpression.nullLiteral());
+      final ColumnType columnType = 
Calcites.getColumnTypeForRelDataType(rexNode.getType());
+      final ExpressionType expressionType = columnType == null ? null : 
ExpressionType.fromColumnTypeStrict(columnType);
+      retVal = ExprEval.ofType(expressionType, null);
     } else if (SqlTypeName.INT_TYPES.contains(sqlTypeName)) {
       final Number number = (Number) RexLiteral.value(rexNode);
-      return DruidExpression.ofLiteral(
-          columnType,
-          number == null ? DruidExpression.nullLiteral() : 
DruidExpression.longLiteral(number.longValue())
-      );
+      retVal = ExprEval.ofType(ExpressionType.LONG, number == null ? null : 
number.longValue());
     } else if (SqlTypeName.NUMERIC_TYPES.contains(sqlTypeName)) {
       // Numeric, non-INT, means we represent it as a double.
       final Number number = (Number) RexLiteral.value(rexNode);
-      return DruidExpression.ofLiteral(
-          columnType,
-          number == null ? DruidExpression.nullLiteral() : 
DruidExpression.doubleLiteral(number.doubleValue())
-      );
+      retVal = ExprEval.ofType(ExpressionType.DOUBLE, number == null ? null : 
number.doubleValue());
     } else if (SqlTypeFamily.INTERVAL_DAY_TIME == sqlTypeName.getFamily()) {
       // Calcite represents DAY-TIME intervals in milliseconds.
       final long milliseconds = ((Number) 
RexLiteral.value(rexNode)).longValue();
-      return DruidExpression.ofLiteral(columnType, 
DruidExpression.longLiteral(milliseconds));
+      retVal = ExprEval.ofType(ExpressionType.LONG, milliseconds);
     } else if (SqlTypeFamily.INTERVAL_YEAR_MONTH == sqlTypeName.getFamily()) {
       // Calcite represents YEAR-MONTH intervals in months.
       final long months = ((Number) RexLiteral.value(rexNode)).longValue();
-      return DruidExpression.ofLiteral(columnType, 
DruidExpression.longLiteral(months));
+      retVal = ExprEval.ofType(ExpressionType.LONG, months);
     } else if (SqlTypeName.STRING_TYPES.contains(sqlTypeName)) {
-      return DruidExpression.ofStringLiteral(RexLiteral.stringValue(rexNode));
+      final String s = RexLiteral.stringValue(rexNode);
+      retVal = ExprEval.ofType(ExpressionType.STRING, s);
+      isEmptyString = s != null && s.isEmpty();
     } else if (SqlTypeName.TIMESTAMP == sqlTypeName || SqlTypeName.DATE == 
sqlTypeName) {
-      if (RexLiteral.isNullLiteral(rexNode)) {
-        return DruidExpression.ofLiteral(columnType, 
DruidExpression.nullLiteral());
-      } else {
-        return DruidExpression.ofLiteral(
-            columnType,
-            DruidExpression.longLiteral(
-                Calcites.calciteDateTimeLiteralToJoda(rexNode, 
plannerContext.getTimeZone()).getMillis()
-            )
-        );
-      }
-    } else if (SqlTypeName.BOOLEAN == sqlTypeName) {
-      return DruidExpression.ofLiteral(
-          columnType,
-          DruidExpression.longLiteral(RexLiteral.booleanValue(rexNode) ? 1 : 0)
+      retVal = ExprEval.ofType(
+          ExpressionType.LONG,
+          Calcites.calciteDateTimeLiteralToJoda(rexNode, 
plannerContext.getTimeZone()).getMillis()
       );
+    } else if (SqlTypeName.BOOLEAN == sqlTypeName) {
+      retVal = ExprEval.ofType(ExpressionType.LONG, 
RexLiteral.booleanValue(rexNode) ? 1 : 0);
     } else {
       // Can't translate other literals.
       return null;
     }
+
+    return new ExprEvalWrapper(retVal, isEmptyString);

Review Comment:
   The problem I'm trying to solve with `ExprEvalWrapper` is that in 
replace-with-default mode, there is no way to have a constant `Expr` or 
`ExprEval` that contains an empty string. Both `StringExpr` and 
`StringExprEval` convert empty strings to nulls in their constructors. So the 
type `ExprEval` cannot be used here, it has to be some kind of wrapper.
   
   Another possible approach is to extend `ExprEval` with a special 
`EmptyStringExprEval` that is only used by the SQL planner, and return that 
whenever we're in default-value mode and the `trueValue` (or `actualValue`) is 
an empty string. But I didn't do it that way, because that seemed even worse 
somehow.



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