zhtaoxiang commented on code in PR #11545: URL: https://github.com/apache/pinot/pull/11545#discussion_r1320014483
########## 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: (See description of #11518): Without this, `select count(*) from table where aFloatColumn = 0.05` will be converted to `select count(*) from table where CAST(aFloatColumn as "DOUBLE") = 0.05` in multi-stage engine. Pinot servers can handle `aFloatColumn = 0.05` correctly if the column is either float or double: it will treat 0.05 as the right data type since it is a string. However, if the column is float, pinot servers cannot handle `CAST(aFloatColumn as "DOUBLE") = 0.05` correctly: if the stored value is float 0.05, it will be converted to double 0.05000000074505806 then compare with double 0.05 (converted from string). This PR makes the multi-stage engine behaves the same as the v1 engine. > If we really need a patch to solve the issue, I'm ok with the change, but I would say that the correct solution in order to evaluate aFloatColumn = 0.05 would be to translate it to eqFloat(aFloatColumn, parseAsFloat(0.05) 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. There may be other similar issue, we need a longer discussion to see how we solve all issues. This PR solves the obvious issue. -- 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
