yashmayya commented on code in PR #19948:
URL: https://github.com/apache/pinot/pull/19948#discussion_r4350833090
##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotEvaluateLiteralRule.java:
##########
@@ -386,22 +393,44 @@ private static boolean isUnsignedIntegerType(SqlTypeName
sqlTypeName) {
}
}
+ /// Returns the value that the runtime passes to a function for the literal
(see `FunctionOperand`): the value from
+ /// [RexExpressionUtils#fromRexLiteral], converted with
[ColumnDataType#toExternal]. For example, an INTEGER literal
+ /// becomes an `Integer` (Calcite stores a `BigDecimal`), and a TIMESTAMP
literal becomes a `Timestamp`.
@Nullable
- private static Object getLiteralValue(RexLiteral rexLiteral) {
- Object value = rexLiteral.getValue();
- if (value instanceof NlsString) {
- // STRING
- return ((NlsString) value).getValue();
- } else if (value instanceof GregorianCalendar) {
- // TIMESTAMP
- return ((GregorianCalendar) value).getTimeInMillis();
- } else if (value instanceof ByteString) {
- // BYTES
- return ((ByteString) value).getBytes();
- } else if (value instanceof TimeUnitRange) {
- return ((TimeUnitRange) value).name();
- } else {
- return value;
+ private static Object getArgumentValue(RexLiteral rexLiteral, ColumnDataType
argumentType) {
+ if (argumentType == ColumnDataType.OBJECT && rexLiteral.getTypeName() !=
SqlTypeName.SYMBOL) {
+ // The runtime cannot represent this literal (for example an INTERVAL),
so keep the value that Calcite stores
+ return rexLiteral.getValue();
+ }
+ if (canExceedArgumentType(rexLiteral, argumentType)) {
+ // Keep the exact value, which the conversion to INT or LONG can wrap
around
+ return rexLiteral.getValue();
+ }
+ RexExpression.Literal literal =
RexExpressionUtils.fromRexLiteral(rexLiteral);
+ Object value = literal.getValue();
+ return value != null ? literal.getDataType().toExternal(value) : null;
+ }
+
+ /// Returns whether the literal is a DECIMAL whose type can hold numbers
beyond the range of its INT or LONG argument
+ /// type. The planner gives DECIMAL(p, 0) the type INT or LONG by its
precision only, for example INT to
+ /// DECIMAL(10, 0), and LONG to an integer literal beyond the BIGINT range.
The decision depends on the type, not on
+ /// the value, so that the literals of one type (for example the elements of
an array) get the same Java class.
+ private static boolean canExceedArgumentType(RexLiteral rexLiteral,
ColumnDataType argumentType) {
Review Comment:
Added a comment there in d28d8d2c3c: a fitting value of such a DECIMAL type
stays a BigDecimal, unlike at runtime, but no value wraps around.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
pinot-query-runtime/src/test/resources/queries/LiteralExpressions.json:
##########
@@ -36,5 +36,38 @@
]
}
]
+ },
+ "literal_function_evaluation": {
+ "comments": "The planner evaluates a function call on literals. It must
pass the literals as the Java values that the runtime passes for the same
values, e.g. an Integer for an INT literal rather than the BigDecimal that
Calcite stores, or functions that dispatch on the value class return other
results. Each query compares a call on a literal with the same call on column
values, which runs above the leaf stage because of the GROUP BY.",
+ "tables": {
+ "tbl": {
+ "schema": [
+ {"name": "intCol", "type": "INT"},
+ {"name": "longCol", "type": "LONG"},
+ {"name": "doubleCol", "type": "DOUBLE"}
+ ],
+ "inputs": [
+ [1, 10000000001, 1.25],
+ [5, 10000000005, 1.5]
+ ]
+ }
+ },
+ "queries": [
+ {
+ "description": "Sketches of an INT literal",
+ "sql": "SELECT intCol, toBase64(toThetaSketch(5)) =
toBase64(toThetaSketch(intCol)), toBase64(toHLL(5)) = toBase64(toHLL(intCol)),
toBase64(toCpcSketch(5)) = toBase64(toCpcSketch(intCol)), toBase64(toULL(5)) =
toBase64(toULL(intCol)), toBase64(toIntegerSumTupleSketch(5, 1)) =
toBase64(toIntegerSumTupleSketch(intCol, 1)) FROM (SELECT intCol FROM {tbl}
GROUP BY intCol) t",
+ "h2Sql": "SELECT intCol, intCol = 5, intCol = 5, intCol = 5, intCol =
5, intCol = 5 FROM (SELECT intCol FROM {tbl} GROUP BY intCol) t"
+ },
+ {
+ "description": "Sketch of a BIGINT literal",
+ "sql": "SELECT longCol, toBase64(toThetaSketch(10000000005)) =
toBase64(toThetaSketch(longCol)) FROM (SELECT longCol FROM {tbl} GROUP BY
longCol) t",
+ "h2Sql": "SELECT longCol, longCol = 10000000005 FROM (SELECT longCol
FROM {tbl} GROUP BY longCol) t"
+ },
+ {
+ "description": "Sketch of a DECIMAL literal, which the runtime reads
as a DOUBLE",
Review Comment:
Added two plan tests in d28d8d2c3c (`LiteralEvaluationPlans.json`):
`jsonFormat(1.50)` folds to '1.5', and `toThetaSketch(TIMESTAMP '...')` fails
at planning with the runtime error. Both fail with the old rule.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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]
