siddharthteotia commented on a change in pull request #6927:
URL: https://github.com/apache/incubator-pinot/pull/6927#discussion_r640777126
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/NumericalFilterOptimizer.java
##########
@@ -271,7 +242,211 @@ private Expression rewrite(Expression equals, Expression
lhs, Expression rhs, Sc
break;
}
}
+ }
}
return equals;
}
+
+ /**
+ * Rewrite expressions of form "column > literal", "column >= literal",
"column < literal", and "column <= literal"
+ * to ensure that RHS literal is the same datatype as LHS column.
+ */
+ private static Expression rewriteRangeExpression(Expression range,
FilterKind kind, Expression lhs, Expression rhs, Schema schema) {
+ // Get column data type.
+ FieldSpec.DataType dataType =
schema.getFieldSpecFor(lhs.getIdentifier().getName()).getDataType();
+
+ switch (rhs.getLiteral().getSetField()) {
+ case SHORT_VALUE:
+ case INT_VALUE:
+ // No rewrites needed since SHORT and INT conversion to numeric column
types (INT, LONG, FLOAT, and DOUBLE) is
+ // lossless and will be implicitly handled on the server side.
+ break;
+ case LONG_VALUE: {
+ long actual = rhs.getLiteral().getLongValue();
+ switch (dataType) {
+ case INT: {
+ int converted = (int) actual;
+ int comparison = Long.compare(actual, converted);
+ if (comparison > 0) {
+ // Literal value is greater than the bounds of INT. > and >=
expressions will always be false because an
+ // INT column can never have a value greater than
Integer.MAX_VALUE. < and <= expressions will always be
+ // true, because an INT column will always have values less than
Integer.MIN_VALUE.
+ setExpressionToBoolean(range, kind == FilterKind.LESS_THAN ||
kind == FilterKind.LESS_THAN_OR_EQUAL);
+ } else if (comparison < 0) {
+ // Literal value is less than the bounds of INT. > and >=
expressions will always be true because an
+ // INT column will always have a value greater than or equal to
Integer.MIN_VALUE. < and <= expressions
+ // will always be false, because an INT column will never have
values less than Integer.MIN_VALUE.
+ setExpressionToBoolean(range, kind == FilterKind.GREATER_THAN ||
kind == FilterKind.GREATER_THAN_OR_EQUAL);
+ } else {
+ // Long literal value falls within the bounds of INT column,
server will successfully convert the literal
+ // value when needed.
+ }
+ break;
+ }
+ case FLOAT: {
+ // Since we are converting a long value to float, float value will
never be out of bounds (i.e -Infinity
+ // or +Infinity).
+ float converted = (float) actual;
+ int comparison =
BigDecimal.valueOf(actual).compareTo(BigDecimal.valueOf(converted));
+
+ // Rewrite range operator
+ rewriteRangeOperator(range, kind, comparison);
+
+ // Rewrite range literal
+ rhs.getLiteral().setDoubleValue(converted);
+ break;
+ }
+ case DOUBLE: {
+ // long to double conversion is always within bounds of double
datatype, but the conversion can be lossy.
+ double converted = (double) actual;
Review comment:
I don't think we were able to prove this earlier when we did a small
testing exercise offline. Unless we can give an example of what is meant by
lossy, I don't think we should add this comment
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
