Xikui Wang has submitted this change and it was merged. Change subject: [NO ISSUE][COMP] Prevent broadcast hint changes left-outer-join semantic ......................................................................
[NO ISSUE][COMP] Prevent broadcast hint changes left-outer-join semantic - user model changes: no - storage format changes: no - interface changes: no Details: The current way of handling broadcast hint can cause a problem in a certain case. When we have a subquery containing join, the inline subplan rule will transform it into a left-outer-join. If the join in the subquery has a broadcast hint and the broadcast side is left, then the JoinUtils will switch the two branches blindly which can break the semantic of the left-outer-join. Change-Id: I522b5f1edf35a1c46f2e2ef1b265049d3c18a575 Reviewed-on: https://asterix-gerrit.ics.uci.edu/2721 Sonar-Qube: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> Contrib: Jenkins <[email protected]> Integration-Tests: Jenkins <[email protected]> Reviewed-by: Dmitry Lychagin <[email protected]> --- A asterixdb/asterix-app/src/test/resources/optimizerts/queries/joins/nested_query_with_bcast.sqlpp A asterixdb/asterix-app/src/test/resources/optimizerts/results/joins/nested_query_with_bcast.plan M hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/util/JoinUtils.java 3 files changed, 92 insertions(+), 7 deletions(-) Approvals: Anon. E. Moose #1000171: Jenkins: Verified; No violations found; ; Verified Dmitry Lychagin: Looks good to me, approved diff --git a/asterixdb/asterix-app/src/test/resources/optimizerts/queries/joins/nested_query_with_bcast.sqlpp b/asterixdb/asterix-app/src/test/resources/optimizerts/queries/joins/nested_query_with_bcast.sqlpp new file mode 100644 index 0000000..8e7a6e0 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/optimizerts/queries/joins/nested_query_with_bcast.sqlpp @@ -0,0 +1,41 @@ +/* + * 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 Country as open { + country_code : string, + country_name : string +}; +create type TweetType as open { + id : int64, + country : string +}; + +create type StoredTweetType as open { + tid : uuid +}; + +create dataset targetDataset(StoredTweetType) primary key tid autogenerated; +create dataset countryDataset(Country) primary key country_code; +create dataset tweetDataset(TweetType) primary key id; + +insert into targetDataset (select object_merge(x, {"full-country" : (select value c.country_name from countryDataset c where c.country_code /*+ bcast */ = x.country )}) from tweetDataset x); \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/optimizerts/results/joins/nested_query_with_bcast.plan b/asterixdb/asterix-app/src/test/resources/optimizerts/results/joins/nested_query_with_bcast.plan new file mode 100644 index 0000000..58af75f --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/optimizerts/results/joins/nested_query_with_bcast.plan @@ -0,0 +1,39 @@ +-- COMMIT |PARTITIONED| + -- STREAM_PROJECT |PARTITIONED| + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + -- INSERT_DELETE |PARTITIONED| + -- HASH_PARTITION_EXCHANGE [$$31] |PARTITIONED| + -- ASSIGN |PARTITIONED| + -- STREAM_PROJECT |PARTITIONED| + -- ASSIGN |PARTITIONED| + -- STREAM_PROJECT |PARTITIONED| + -- ASSIGN |PARTITIONED| + -- STREAM_PROJECT |PARTITIONED| + -- ASSIGN |PARTITIONED| + -- STREAM_PROJECT |PARTITIONED| + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + -- PRE_CLUSTERED_GROUP_BY[$$34] |PARTITIONED| + { + -- AGGREGATE |LOCAL| + -- STREAM_SELECT |LOCAL| + -- NESTED_TUPLE_SOURCE |LOCAL| + } + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + -- STABLE_SORT [$$34(ASC)] |PARTITIONED| + -- HASH_PARTITION_EXCHANGE [$$34] |PARTITIONED| + -- STREAM_PROJECT |PARTITIONED| + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + -- HYBRID_HASH_JOIN [$$37][$$35] |PARTITIONED| + -- HASH_PARTITION_EXCHANGE [$$37] |PARTITIONED| + -- ASSIGN |PARTITIONED| + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + -- DATASOURCE_SCAN |PARTITIONED| + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + -- EMPTY_TUPLE_SOURCE |PARTITIONED| + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + -- STREAM_PROJECT |PARTITIONED| + -- ASSIGN |PARTITIONED| + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + -- DATASOURCE_SCAN |PARTITIONED| + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + -- EMPTY_TUPLE_SOURCE |PARTITIONED| diff --git a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/util/JoinUtils.java b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/util/JoinUtils.java index 555d468..98e48fd 100644 --- a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/util/JoinUtils.java +++ b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/util/JoinUtils.java @@ -70,15 +70,20 @@ setHashJoinOp(op, JoinPartitioningType.BROADCAST, sideLeft, sideRight, context); break; case LEFT: - Mutable<ILogicalOperator> opRef0 = op.getInputs().get(0); - Mutable<ILogicalOperator> opRef1 = op.getInputs().get(1); - ILogicalOperator tmp = opRef0.getValue(); - opRef0.setValue(opRef1.getValue()); - opRef1.setValue(tmp); - setHashJoinOp(op, JoinPartitioningType.BROADCAST, sideRight, sideLeft, context); + if (op.getJoinKind() == AbstractBinaryJoinOperator.JoinKind.INNER) { + Mutable<ILogicalOperator> opRef0 = op.getInputs().get(0); + Mutable<ILogicalOperator> opRef1 = op.getInputs().get(1); + ILogicalOperator tmp = opRef0.getValue(); + opRef0.setValue(opRef1.getValue()); + opRef1.setValue(tmp); + setHashJoinOp(op, JoinPartitioningType.BROADCAST, sideRight, sideLeft, context); + } else { + setHashJoinOp(op, JoinPartitioningType.PAIRWISE, sideLeft, sideRight, context); + } break; default: - setHashJoinOp(op, JoinPartitioningType.PAIRWISE, sideLeft, sideRight, context); + // This should never happen + throw new IllegalStateException(side.toString()); } } } else { -- To view, visit https://asterix-gerrit.ics.uci.edu/2721 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: I522b5f1edf35a1c46f2e2ef1b265049d3c18a575 Gerrit-PatchSet: 6 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Xikui Wang <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Xikui Wang <[email protected]>
