Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: None
-
Fix Version/s: 1.5.0
-
Component/s: None
-
Labels:None
Description
We need to improve ReduceExpressionRule to deal with multiple "equals to literal" in predicate. In ReduceExpressionRule.java, L396-397:
final ImmutableMap<RexNode, RexLiteral> constants =
predicateConstants(predicates);
The query is
select * from src where (key='12' and key is null);
Here 'key' is a string type and it is one of the columns of 'src' table.
Then we will have predicates
[=($0, '12'), isnull($0)]
which is the input of the
predicateConstants, and the function will return "{$0='12'}"
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
Julian Hyde, could you please take a look? Thanks.
I reviewed your changes. I tidied up by gathering the code into a method, and using a multimap to defer detecting duplicates. I also added a test case. The test case shows that if you have a non-equi constraint, say 'mgr is null' then it prevents any constant reduction occurring. That is not OK. Constraints which are not 'column = constant' (and that includes constraints 'column = constant1 and column = constant2') should be ignored, but they should not prevent constant reduction.
Can you build on the work in https://github.com/julianhyde/incubator-calcite/tree/935-reduce-multiple-constraints and make sure that testReduceConstantsDup2 reduces empno to 10 and ignores the constraints on deptno and mgr. (If you reduce mgr to null I wouldn't object.)
Hi Julian, Thanks for your efforts. I thought about the multi map for a while and I changed it back to basic map due to 2 reasons (1) as RexLiteral is immutable too, a constant '10' is different from a constant '10'. Thus there will be two values of '10'. (2) We only need a single literal value. Storing multiple values in a multi-map is not that useful. The new patch is in https://github.com/julianhyde/incubator-calcite/commit/ee02796a4f7a1c26f040657bb58026fa03da8666 and I already sent u a pull request. Plz let me know what you think. Thanks.
Fixed in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/690faa55. Thanks for the patch, Pengcheng Xiong!
Resolved in release 1.5.0 (2015-11-10)
I plan to submit a patch myself soon.