>From Ali Alsuliman <[email protected]>: Ali Alsuliman has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18864 )
Change subject: [ASTERIXDB-3501][COMP] Redundant listify in the plan ...................................................................... [ASTERIXDB-3501][COMP] Redundant listify in the plan - user model changes: no - storage format changes: no - interface changes: no Ext-ref: MB-63443 Change-Id: Ic80c47080313b85026a03d07be2cf75a079782ab Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18864 Tested-by: Jenkins <[email protected]> Reviewed-by: Peeyush Gupta <[email protected]> Reviewed-by: Ali Alsuliman <[email protected]> --- M asterixdb/asterix-app/src/test/resources/optimizerts/results/subquery/in_correlated_ps.plan A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/remove_listify/remove_listify.3.query.sqlpp A asterixdb/asterix-app/src/test/resources/optimizerts/queries/remove_listify.sqlpp M asterixdb/asterix-app/src/test/resources/optimizerts/results/subquery/in_correlated.plan M hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorManipulationUtil.java M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/CancelUnnestWithNestedListifyRule.java M asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/remove_listify/remove_listify.1.ddl.sqlpp A asterixdb/asterix-app/src/test/resources/optimizerts/results/remove_listify.plan A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/remove_listify/remove_listify.2.update.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/results/misc/remove_listify/remove_listify.3.adm 11 files changed, 318 insertions(+), 34 deletions(-) Approvals: Ali Alsuliman: Looks good to me, approved Peeyush Gupta: Looks good to me, but someone else must approve Jenkins: Verified Anon. E. Moose #1000171: diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/CancelUnnestWithNestedListifyRule.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/CancelUnnestWithNestedListifyRule.java index 366da5a..93c4ea2 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/CancelUnnestWithNestedListifyRule.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/CancelUnnestWithNestedListifyRule.java @@ -28,6 +28,7 @@ import org.apache.commons.lang3.mutable.Mutable; import org.apache.commons.lang3.mutable.MutableObject; import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException; +import org.apache.hyracks.algebricks.common.utils.ListSet; import org.apache.hyracks.algebricks.core.algebra.base.ILogicalExpression; import org.apache.hyracks.algebricks.core.algebra.base.ILogicalOperator; import org.apache.hyracks.algebricks.core.algebra.base.ILogicalPlan; @@ -119,6 +120,10 @@ AbstractLogicalOperator op = (AbstractLogicalOperator) opRef.getValue(); VariableUtilities.getUsedVariables(op, varSet); if (op.hasNestedPlans()) { + // Variables used by the parent operators should be live at op. + Set<LogicalVariable> localLiveVars = new ListSet<>(); + VariableUtilities.getLiveVariables(op, localLiveVars); + varSet.retainAll(localLiveVars); AbstractOperatorWithNestedPlans aonp = (AbstractOperatorWithNestedPlans) op; for (ILogicalPlan p : aonp.getNestedPlans()) { for (Mutable<ILogicalOperator> r : p.getRoots()) { @@ -198,8 +203,9 @@ return false; } - if (OperatorManipulationUtil.ancestorOfOperators(agg, ImmutableSet.of(LogicalOperatorTag.LIMIT, - LogicalOperatorTag.ORDER, LogicalOperatorTag.GROUP, LogicalOperatorTag.DISTINCT))) { + if (OperatorManipulationUtil.ancestorOfOperatorsExcludeCurrent(agg, + ImmutableSet.of(LogicalOperatorTag.LIMIT, LogicalOperatorTag.ORDER, LogicalOperatorTag.GROUP, + LogicalOperatorTag.DISTINCT, LogicalOperatorTag.AGGREGATE))) { return false; } diff --git a/asterixdb/asterix-app/src/test/resources/optimizerts/queries/remove_listify.sqlpp b/asterixdb/asterix-app/src/test/resources/optimizerts/queries/remove_listify.sqlpp new file mode 100644 index 0000000..d6475f3 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/optimizerts/queries/remove_listify.sqlpp @@ -0,0 +1,48 @@ +/* + * 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. + */ + +drop dataverse test if exists; +create dataverse test; + +use test; + + +create type dt as +{ + uid : integer +}; + +create dataset collection1(dt) primary key uid; + +SET `rewrite_or_as_join` "false"; +WITH table23 AS( + SELECT f1, + f2 + FROM collection1 b), +table22 AS( + SELECT f1, + ( + SELECT COUNT(1) FILTER(WHERE b.f2>=a.f2-5 AND b.f2<=a.f2) AS counts + FROM table23 b + WHERE b.f1=a.f1 + )[0] counts + FROM table23 a) +SELECT + value a +FROM table22 a; \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/optimizerts/results/remove_listify.plan b/asterixdb/asterix-app/src/test/resources/optimizerts/results/remove_listify.plan new file mode 100644 index 0000000..96d4248 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/optimizerts/results/remove_listify.plan @@ -0,0 +1,36 @@ +-- DISTRIBUTE_RESULT |PARTITIONED| + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + -- STREAM_PROJECT |PARTITIONED| + -- ASSIGN |PARTITIONED| + -- STREAM_PROJECT |PARTITIONED| + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + -- PRE_CLUSTERED_GROUP_BY[$$141] |PARTITIONED| + { + -- AGGREGATE |LOCAL| + -- AGGREGATE |LOCAL| + -- STREAM_SELECT |LOCAL| + -- ASSIGN |LOCAL| + -- NESTED_TUPLE_SOURCE |LOCAL| + } + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + -- STABLE_SORT [$$141(ASC)] |PARTITIONED| + -- HASH_PARTITION_EXCHANGE [$$141] |PARTITIONED| + -- STREAM_PROJECT |PARTITIONED| + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + -- HYBRID_HASH_JOIN [$$146][$$148] |PARTITIONED| + -- HASH_PARTITION_EXCHANGE [$$146] |PARTITIONED| + -- STREAM_PROJECT |PARTITIONED| + -- ASSIGN |PARTITIONED| + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + -- DATASOURCE_SCAN (test.collection1) |PARTITIONED| + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + -- EMPTY_TUPLE_SOURCE |PARTITIONED| + -- HASH_PARTITION_EXCHANGE [$$148] |PARTITIONED| + -- STREAM_PROJECT |PARTITIONED| + -- ASSIGN |PARTITIONED| + -- ASSIGN |PARTITIONED| + -- STREAM_PROJECT |PARTITIONED| + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + -- DATASOURCE_SCAN (test.collection1) |PARTITIONED| + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + -- EMPTY_TUPLE_SOURCE |PARTITIONED| \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/optimizerts/results/subquery/in_correlated.plan b/asterixdb/asterix-app/src/test/resources/optimizerts/results/subquery/in_correlated.plan index 540e821..42c8f8f 100644 --- a/asterixdb/asterix-app/src/test/resources/optimizerts/results/subquery/in_correlated.plan +++ b/asterixdb/asterix-app/src/test/resources/optimizerts/results/subquery/in_correlated.plan @@ -12,20 +12,13 @@ { -- AGGREGATE |LOCAL| -- STREAM_SELECT |LOCAL| - -- UNNEST |LOCAL| - -- MICRO_PRE_CLUSTERED_GROUP_BY[] |LOCAL| - { - -- AGGREGATE |LOCAL| - -- STREAM_SELECT |LOCAL| - -- NESTED_TUPLE_SOURCE |LOCAL| - } - -- NESTED_TUPLE_SOURCE |LOCAL| + -- NESTED_TUPLE_SOURCE |LOCAL| } -- ONE_TO_ONE_EXCHANGE |PARTITIONED| -- HYBRID_HASH_JOIN [$$49][$$48] |PARTITIONED| -- ONE_TO_ONE_EXCHANGE |PARTITIONED| - -- STREAM_SELECT |PARTITIONED| - -- ASSIGN |PARTITIONED| + -- STREAM_PROJECT |PARTITIONED| + -- STREAM_SELECT |PARTITIONED| -- ONE_TO_ONE_EXCHANGE |PARTITIONED| -- DATASOURCE_SCAN (test.Customers) |PARTITIONED| -- ONE_TO_ONE_EXCHANGE |PARTITIONED| @@ -37,4 +30,4 @@ -- ONE_TO_ONE_EXCHANGE |PARTITIONED| -- DATASOURCE_SCAN (test.Orders) |PARTITIONED| -- ONE_TO_ONE_EXCHANGE |PARTITIONED| - -- EMPTY_TUPLE_SOURCE |PARTITIONED| + -- EMPTY_TUPLE_SOURCE |PARTITIONED| \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/optimizerts/results/subquery/in_correlated_ps.plan b/asterixdb/asterix-app/src/test/resources/optimizerts/results/subquery/in_correlated_ps.plan index 6080780..85eb2da 100644 --- a/asterixdb/asterix-app/src/test/resources/optimizerts/results/subquery/in_correlated_ps.plan +++ b/asterixdb/asterix-app/src/test/resources/optimizerts/results/subquery/in_correlated_ps.plan @@ -16,20 +16,13 @@ { -- AGGREGATE |LOCAL| -- STREAM_SELECT |LOCAL| - -- UNNEST |LOCAL| - -- MICRO_PRE_CLUSTERED_GROUP_BY[] |LOCAL| - { - -- AGGREGATE |LOCAL| - -- STREAM_SELECT |LOCAL| - -- NESTED_TUPLE_SOURCE |LOCAL| - } - -- NESTED_TUPLE_SOURCE |LOCAL| + -- NESTED_TUPLE_SOURCE |LOCAL| } -- ONE_TO_ONE_EXCHANGE |PARTITIONED| -- HYBRID_HASH_JOIN [$$49][$$48] |PARTITIONED| -- ONE_TO_ONE_EXCHANGE |PARTITIONED| - -- STREAM_SELECT |PARTITIONED| - -- ASSIGN |PARTITIONED| + -- STREAM_PROJECT |PARTITIONED| + -- STREAM_SELECT |PARTITIONED| -- ONE_TO_ONE_EXCHANGE |PARTITIONED| -- DATASOURCE_SCAN (test.Customers) |PARTITIONED| -- ONE_TO_ONE_EXCHANGE |PARTITIONED| @@ -56,20 +49,13 @@ { -- AGGREGATE |LOCAL| -- STREAM_SELECT |LOCAL| - -- UNNEST |LOCAL| - -- MICRO_PRE_CLUSTERED_GROUP_BY[] |LOCAL| - { - -- AGGREGATE |LOCAL| - -- STREAM_SELECT |LOCAL| - -- NESTED_TUPLE_SOURCE |LOCAL| - } - -- NESTED_TUPLE_SOURCE |LOCAL| + -- NESTED_TUPLE_SOURCE |LOCAL| } -- ONE_TO_ONE_EXCHANGE |PARTITIONED| -- HYBRID_HASH_JOIN [$$49][$$48] |PARTITIONED| -- ONE_TO_ONE_EXCHANGE |PARTITIONED| - -- STREAM_SELECT |PARTITIONED| - -- ASSIGN |PARTITIONED| + -- STREAM_PROJECT |PARTITIONED| + -- STREAM_SELECT |PARTITIONED| -- ONE_TO_ONE_EXCHANGE |PARTITIONED| -- DATASOURCE_SCAN (test.Customers) |PARTITIONED| -- ONE_TO_ONE_EXCHANGE |PARTITIONED| @@ -81,4 +67,4 @@ -- ONE_TO_ONE_EXCHANGE |PARTITIONED| -- DATASOURCE_SCAN (test.Orders) |PARTITIONED| -- ONE_TO_ONE_EXCHANGE |PARTITIONED| - -- EMPTY_TUPLE_SOURCE |PARTITIONED| + -- EMPTY_TUPLE_SOURCE |PARTITIONED| \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/remove_listify/remove_listify.1.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/remove_listify/remove_listify.1.ddl.sqlpp new file mode 100644 index 0000000..498f374 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/remove_listify/remove_listify.1.ddl.sqlpp @@ -0,0 +1,30 @@ +/* + * 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. + */ + +drop dataverse test if exists; +create dataverse test; + +use test; + +create type dt as +{ + uid : integer +}; + +create dataset collection1(dt) primary key uid; \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/remove_listify/remove_listify.2.update.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/remove_listify/remove_listify.2.update.sqlpp new file mode 100644 index 0000000..ac2cc1b --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/remove_listify/remove_listify.2.update.sqlpp @@ -0,0 +1,100 @@ +/* + * 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 test; + +insert into collection1 +([ + + { + "uid":1, + "f1":101, + "f2": 24 + }, + { + "uid":2, + "f1":102, + "f2": 25 + }, + { + "uid":3, + "f1":102, + "f2": 25 + }, + { + "uid":4, + "f1":102, + "f2": 31 + }, + { + "uid":5, + "f1":103, + "f2": 24 + }, + { + "uid":6, + "f1":103, + "f2": 30 + }, + { + "uid":7, + "f1":103, + "f2": 31 + }, + { + "uid":8, + "f1":103, + "f2": 35 + }, + { + "uid":9, + "f1":103, + "f2": 41 + }, + { + "uid":10, + "f1":104, + "f2": 42 + }, + { + "uid":11, + "f1":104, + "f2": 24 + }, + { + "uid":12, + "f1":104, + "f2": 25 + }, + { + "uid":13, + "f1":105, + "f2": 24 + }, + { + "uid":14, + "f1":105, + "f2": 25 + }, + { + "uid":15, + "f1":105, + "f2": "40" + } +]) diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/remove_listify/remove_listify.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/remove_listify/remove_listify.3.query.sqlpp new file mode 100644 index 0000000..8652bba --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/remove_listify/remove_listify.3.query.sqlpp @@ -0,0 +1,38 @@ +/* + * 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 test; + +SET `rewrite_or_as_join` "false"; +WITH table23 AS( + SELECT f1, + f2 + FROM collection1 b), +table22 AS( + SELECT f1, + ( + SELECT COUNT(1) FILTER(WHERE b.f2>=a.f2-5 AND b.f2<=a.f2) AS counts + FROM table23 b + WHERE b.f1=a.f1 + )[0] obj + FROM table23 a) +SELECT + a.f1, a.obj.counts +FROM table22 a +order by a.f1, a.obj.counts; \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/remove_listify/remove_listify.3.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/remove_listify/remove_listify.3.adm new file mode 100644 index 0000000..a4142d4 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/remove_listify/remove_listify.3.adm @@ -0,0 +1,15 @@ +{ "counts": 1, "f1": 101 } +{ "counts": 1, "f1": 102 } +{ "counts": 2, "f1": 102 } +{ "counts": 2, "f1": 102 } +{ "counts": 1, "f1": 103 } +{ "counts": 1, "f1": 103 } +{ "counts": 1, "f1": 103 } +{ "counts": 2, "f1": 103 } +{ "counts": 3, "f1": 103 } +{ "counts": 1, "f1": 104 } +{ "counts": 1, "f1": 104 } +{ "counts": 2, "f1": 104 } +{ "counts": 0, "f1": 105 } +{ "counts": 1, "f1": 105 } +{ "counts": 2, "f1": 105 } \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml index 9d75c8d..9e5f634 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml @@ -7315,6 +7315,11 @@ <output-dir compare="Text">query-ASTERIXDB-3316</output-dir> </compilation-unit> </test-case> + <test-case FilePath="misc"> + <compilation-unit name="remove_listify"> + <output-dir compare="Text">remove_listify</output-dir> + </compilation-unit> + </test-case> </test-group> <test-group name="multipart-dataverse"> <test-case FilePath="multipart-dataverse"> diff --git a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorManipulationUtil.java b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorManipulationUtil.java index 3457751..2413ed2 100644 --- a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorManipulationUtil.java +++ b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorManipulationUtil.java @@ -361,6 +361,15 @@ return false; } + public static boolean ancestorOfOperatorsExcludeCurrent(ILogicalOperator op, Set<LogicalOperatorTag> tags) { + for (Mutable<ILogicalOperator> children : op.getInputs()) { + if (ancestorOfOperators(children.getValue(), tags)) { + return true; + } + } + return false; + } + /** * Returns all descendants of an operator that are leaf operators * -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18864 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: neo Gerrit-Change-Id: Ic80c47080313b85026a03d07be2cf75a079782ab Gerrit-Change-Number: 18864 Gerrit-PatchSet: 4 Gerrit-Owner: Peeyush Gupta <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Peeyush Gupta <[email protected]> Gerrit-MessageType: merged
