gianm commented on code in PR #13037:
URL: https://github.com/apache/druid/pull/13037#discussion_r965404372
##########
sql/src/main/java/org/apache/druid/sql/calcite/expression/DruidExpression.java:
##########
@@ -104,9 +104,24 @@ private static String escape(final String s)
return escaped.toString();
}
- public static String numberLiteral(final Number n)
+ public static String longLiteral(final long n)
{
- return n == null ? nullLiteral() : n.toString();
+ return String.valueOf(n);
+ }
+
+ public static String doubleLiteral(final double n)
+ {
+ final String s = String.valueOf(n);
+
+ // Ensure number is parsed as a double: add ".0" if necessary.
+ for (int i = 0; i < s.length(); i++) {
+ final char c = s.charAt(i);
+ if (!Character.isDigit(c)) {
Review Comment:
I discovered a couple of things.
First: there is no need to do any of this checking, since simply using
`String.valueOf(double)` is enough: it adds the extra `.0` for whole numbers
anyway. The real fix to the bug was replacing `numberLiteral` with
`longLiteral` and `doubleLiteral`, as this forces us to check the desired type,
which means we don't lose the cast. (Previously: we'd lose casts by passing
long `3` to `numberLiteral` even though the desired type was DOUBLE.) So, we
can simplify the code here, which is nice.
Second: I tried to add a test for `Long.MIN_VALUE` and found that we cannot
parse it! It sort of makes sense. To avoid ambiguity between unary minus and
negative literals, we don't actually have negative literals. We represent
"negative literals" as unary minus next to a positive literal of the absolute
value. But! That means `Long.MIN_VALUE` can't be represented, since the
absolute value can't fit into a `long` and therefore can't go into a LongExpr.
I'm going to try to fix this and see where it leads me. This is where the 🐰
comes in…
--
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]
