mihaibudiu commented on code in PR #4301: URL: https://github.com/apache/calcite/pull/4301#discussion_r2041280887
########## core/src/test/resources/sql/test.iq: ########## @@ -0,0 +1,63 @@ +# test.iq - Join query tests +# +# 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. +# +!use post +!set outputformat mysql +!set rules "-CoreRules.INTERSECT_TO_DISTINCT,-EnumerableRules.ENUMERABLE_INTERSECT_RULE,+CoreRules.INTERSECT_TO_SEMI_JOIN" + +# Test one +with t (i) as (values (0), (1)) +select * from t as t1 +intersect +select * from t as t2 where t2.i > 0; ++---+ +| I | ++---+ +| 1 | ++---+ +(1 row) + +!ok + +EnumerableNestedLoopJoin(condition=[IS NOT DISTINCT FROM($0, $1)], joinType=[semi]) Review Comment: you should have two tests after setting the rules, so we can check whether the rules set apply to the end of the file ########## core/src/test/resources/sql/test.iq: ########## @@ -0,0 +1,63 @@ +# test.iq - Join query tests Review Comment: you should probably call this file something else, perhaps planner.iq? the comment is no longer accurate ########## core/src/test/resources/sql/test.iq: ########## @@ -0,0 +1,63 @@ +# test.iq - Join query tests +# +# 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. +# +!use post +!set outputformat mysql +!set rules "-CoreRules.INTERSECT_TO_DISTINCT,-EnumerableRules.ENUMERABLE_INTERSECT_RULE,+CoreRules.INTERSECT_TO_SEMI_JOIN" Review Comment: this doesn't really "set" the rules, it updates them ########## core/src/test/resources/sql/test.iq: ########## @@ -0,0 +1,63 @@ +# test.iq - Join query tests +# +# 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. +# +!use post +!set outputformat mysql +!set rules "-CoreRules.INTERSECT_TO_DISTINCT,-EnumerableRules.ENUMERABLE_INTERSECT_RULE,+CoreRules.INTERSECT_TO_SEMI_JOIN" + +# Test one +with t (i) as (values (0), (1)) +select * from t as t1 +intersect +select * from t as t2 where t2.i > 0; ++---+ +| I | ++---+ +| 1 | ++---+ +(1 row) + +!ok + +EnumerableNestedLoopJoin(condition=[IS NOT DISTINCT FROM($0, $1)], joinType=[semi]) + EnumerableValues(tuples=[[{ 0 }, { 1 }]]) + EnumerableCalc(expr#0=[{inputs}], expr#1=[0], expr#2=[>($t0, $t1)], EXPR$0=[$t0], $condition=[$t2]) + EnumerableValues(tuples=[[{ 0 }, { 1 }]]) +!plan +!set reset-rules "" Review Comment: this should probably be called "!reset rules" ########## testkit/src/main/java/org/apache/calcite/test/QuidemTest.java: ########## @@ -171,6 +182,33 @@ protected void checkRun(String path) throws Exception { int thresholdValue = ((BigDecimal) value).intValue(); closer.add(Prepare.THREAD_INSUBQUERY_THRESHOLD.push(thresholdValue)); } + if (propertyName.equals("rules")) { + parseRules((String) value); + closer.add( + Hook.PLANNER.addThread((Consumer<RelOptPlanner>) + planner -> { + for (RelOptRule rule : addRulesCache) { + planner.addRule(rule); + } + for (RelOptRule rule : removeRulesCache) { + planner.removeRule(rule); + } + })); + } + if (propertyName.equals("reset-rules")) { Review Comment: I am not sure this undoes the effect of the previous. In particular, I don't know what the order of the rules is in the planner, and where in the planner the inserted rules will be Ideally you can save the entire list of planner rules and restore it. I don't know if the hooks allow that ########## testkit/src/main/java/org/apache/calcite/test/QuidemTest.java: ########## @@ -71,6 +78,10 @@ public abstract class QuidemTest { private static final Pattern PATTERN = Pattern.compile("\\.iq$"); + private List<RelOptRule> addRulesCache = new ArrayList<>(); Review Comment: even though these are private, I would still document them. ########## testkit/src/main/java/org/apache/calcite/test/QuidemTest.java: ########## @@ -187,6 +225,41 @@ protected void checkRun(String path) throws Exception { } } + private void parseRules(String value) { + Pattern pattern = Pattern.compile("([+-])(CoreRules|EnumerableRules)\\.(\\w+)"); + Matcher matcher = pattern.matcher(value); + + while (matcher.find()) { + char operation = matcher.group(1).charAt(0); + String ruleSource = matcher.group(2); + String ruleName = matcher.group(3); + + try { + if (ruleSource.equals("CoreRules")) { Review Comment: Maybe this can be the default - if no source is specified, to make the specification of rules shorter -- 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]
