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


##########
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:
   I feel like we are making an exception to literals which are `null` - to 
work like they are true `null`-s in `defaultMode` ; however if you insert a 
`null` into a table - it will act like the empty string....
   
   wouldn't it be possible to handle `null` values in case we are in 
`defaults-mode` as values; by making logic-s like 
[this](https://github.com/apache/druid/pull/16311/files#diff-172d30aafbdf15cef24fbd77b3293d7c215c30a3de6253f5139369ac0e15780aR3766)
 will become restricted with `Nullhandling.sqlCompatible()`?
    
   



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