amrishlal commented on pull request #7394: URL: https://github.com/apache/pinot/pull/7394#issuecomment-912864460
> CalciteSqlParser already rewrites queries to convert WHERE a = b to WHERE a - b = 0. Now we're adding one more optimizer to convert WHERE a - b = 0 to WHERE compare(a,b) = 0. Instead we can directly convert WHERE a = b to WHERE compare(a,b) = 0 >StringPredicateFilterOptimizer code is very similar - not exact duplicate - to CalciteSqlParser.updateComparisonPredicate. With small change in the existing updateComparisonPredicate method, we can fix the bug. `CalciteSqlParser` doesn't know whether columns `a` and `b` are `STRING`, `INTEGER`, or `BYTES`, or any other datatype, so we can't really rewrite `WHERE a = b` into `WHERE COMPARE(a,b) = 0` when `a` and `b` are `STRING` and to `WHERE MINUS(a,b)=0` when `a` and `b` are numbers. Also, we cannot overload `MINUS` function to work on `STRING` since `ScalarFunction` currently cannot be overloaded (there are several issues already open on this as this has implications on query correctness and type handling but I don't think there is a good solution yet). > Basically this is not an optimization, it's fixing a bug in the query rewrite logic. True, but we also need to keep in mind that `QueryOptimizer` isn't really an optimizer, its a rewriter :-) > We don't need to split the rewrite logic in two places. If something changes or a bug surfaces in the rewrite logic, we should only update one place in the code. The rewrite logic is already split into two places (one in `CalciteSqlParser` and other in `QueryOptimizer`). So given the issues, this serves as a good "fix" until we do some major refactoring like 1) using Schema and TableConfig information in CalciteSqlParser in which case it should just be merged with `QueryOptimizer` since QueryOptimizer does that already, or 2) fixing the function overloading and type resolution issues with ScalarFunction). I added some comment about this in `StringPredicateFilterOptimizer` and can add more details if needed? -- 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]
