gortiz commented on code in PR #11545: URL: https://github.com/apache/pinot/pull/11545#discussion_r1321385894
########## pinot-query-planner/src/main/java/org/apache/calcite/rex/PinotRexBuilder.java: ########## @@ -0,0 +1,64 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.calcite.rex; + +import org.apache.calcite.rel.type.RelDataType; +import org.apache.calcite.rel.type.RelDataTypeFactory; +import org.apache.calcite.sql.type.SqlTypeName; + + +public class PinotRexBuilder extends RexBuilder { + public PinotRexBuilder(RelDataTypeFactory typeFactory) { + super(typeFactory); + } + + /** + * Ensures expression is interpreted as a specified type. This is an extension of the standard method, this method + * does not cast expression if its {@link SqlTypeName} is float/real and the desired type is double. + * + * This makes queries like `select count(*) from table where aFloatColumn = 0.05` works correctly in multi-stage query + * engine. + * Without this change, `select count(*) from table where aFloatColumn = 0.05` will be converted to + * `select count(*) from table where CAST(aFloatColumn as "DOUBLE") = 0.05`. While casting from float to double does + * not always produce the same double value as the original float value, this leads to wrong query result. + * + * With this change, the behavior in multi-stage query engine will be the same as the query in v1 query engine. + * + * @param type desired type + * @param node expression + * @param matchNullability whether to correct nullability of specified + * type to match the expression; this usually should + * be true, except for explicit casts which can + * override default nullability + * @return a casted expression or the original expression + */ + @Override + public RexNode ensureType( + RelDataType type, + RexNode node, + boolean matchNullability) { + SqlTypeName typeToCastTo = type.getSqlTypeName(); + SqlTypeName typeToCastFrom = node.getType().getSqlTypeName(); + if (typeToCastTo == SqlTypeName.DOUBLE + && (typeToCastFrom == SqlTypeName.REAL || typeToCastFrom == SqlTypeName.FLOAT)) { Review Comment: > I agree that there are better ways. However, translate it to eqFloat(aFloatColumn, parseAsFloat(0.05) may not be an ideal solution since this is not standard SQL. What do you mean by not being standard sql? I'm just using an invented notation. But it should be the standard `eq`. My point is that in this case where the left hand side of the expression must be a float and the right hand side is a literal (which are parsed as BigDecimal by Calcite) we should transform the right hand side into a float instead of what we are doing here, which is violating the semantics of the method and therefore may have other implications. -- 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: commits-unsubscr...@pinot.apache.org 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
