amrishlal commented on a change in pull request #6927: URL: https://github.com/apache/incubator-pinot/pull/6927#discussion_r640850582
########## 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: Added example in 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