Ali Alsuliman has submitted this change and it was merged. ( https://asterix-gerrit.ics.uci.edu/3399 )
Change subject: [ASTERIXDB-2289][COMP] Fix field access with CASE ...................................................................... [ASTERIXDB-2289][COMP] Fix field access with CASE - user model changes: no - storage format changes: no - interface changes: no Details: This patch fixes field access in the presense CASE and JOIN. This is a scenario where push-down-field-access rule throws an exception if the field access has potentially two sources and it could not push down the field access to left or right branch. Don't throw an exception and just return false (i.e. field access was not pushed) instead of throwing an exception. Change-Id: I911e4e9018c15e8f226e46fa610d222eb2301fcd Reviewed-on: https://asterix-gerrit.ics.uci.edu/3399 Contrib: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Tested-by: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Integration-Tests: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Reviewed-by: Dmitry Lychagin <dmitry.lycha...@couchbase.com> --- M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushFieldAccessRule.java A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.1.ddl.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.2.update.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.3.query.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.4.ddl.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/results/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.3.adm M asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml M hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorPropertiesUtil.java 8 files changed, 162 insertions(+), 24 deletions(-) Approvals: Jenkins: Verified; ; Verified Dmitry Lychagin: Looks good to me, approved Objections: Jenkins: Violations found Anon. E. Moose (1000171): Violations found diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushFieldAccessRule.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushFieldAccessRule.java index 421b00d..c82aa33 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushFieldAccessRule.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushFieldAccessRule.java @@ -84,6 +84,9 @@ if (op.getOperatorTag() != LogicalOperatorTag.ASSIGN) { return false; } + if (!OperatorPropertiesUtil.isMovable(op)) { + return false; + } AssignOperator access = (AssignOperator) op; ILogicalExpression expr = getFirstExpr(access); String finalAnnot; @@ -196,17 +199,17 @@ pushDownFieldAccessRec(opRef2, context, finalAnnot); return true; } - List<LogicalVariable> usedInAccess = new LinkedList<>(); + HashSet<LogicalVariable> usedInAccess = new HashSet<>(); VariableUtilities.getUsedVariables(assignOp, usedInAccess); - List<LogicalVariable> produced2 = new LinkedList<>(); + HashSet<LogicalVariable> produced2 = new HashSet<>(); if (inputOp.getOperatorTag() == LogicalOperatorTag.GROUP) { VariableUtilities.getLiveVariables(inputOp, produced2); } else { VariableUtilities.getProducedVariables(inputOp, produced2); } boolean pushItDown = false; - List<LogicalVariable> inter = new ArrayList<>(usedInAccess); + HashSet<LogicalVariable> inter = new HashSet<>(usedInAccess); if (inter.isEmpty()) { // ground value return false; } @@ -234,8 +237,7 @@ LogicalVariable oldVar = assignOp.getVariables().get(0); VariableReferenceExpression v2Ref = new VariableReferenceExpression(v2); v2Ref.setSourceLocation(g.getSourceLocation()); - g.getDecorList().add(new Pair<LogicalVariable, Mutable<ILogicalExpression>>(oldVar, - new MutableObject<ILogicalExpression>(v2Ref))); + g.getDecorList().add(new Pair<>(oldVar, new MutableObject<>(v2Ref))); changed = true; assignOp.getVariables().set(0, v2); VariableUtilities.substituteVariables(assignOp, m.first, m.second, context); @@ -281,8 +283,7 @@ } } } - throw new CompilationException(ErrorCode.COMPILATION_ERROR, assignOp.getSourceLocation(), - "Field access " + assignOp.getExpressions().get(0).getValue() + " doesn't correspond to any input"); + return false; } else { // check if the accessed field is one of the partitioning key fields. If yes, we can equate the 2 variables if (inputOp.getOperatorTag() == LogicalOperatorTag.DATASOURCESCAN) { diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.1.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.1.ddl.sqlpp new file mode 100644 index 0000000..46e1f02 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.1.ddl.sqlpp @@ -0,0 +1,50 @@ +/* + * 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. + */ + +// testing fix for ASTERIXDB-2289 related to field access with CASE +DROP DATAVERSE TinySocial IF EXISTS; +CREATE DATAVERSE TinySocial; +USE TinySocial; + +CREATE TYPE ChirpUserType AS { + screenName: string, + lang: string, + friendsCount: int, + statusesCount: int, + name: string, + followersCount: int +}; + +CREATE TYPE EmploymentType AS { + organizationName: string, + startDate: date, + endDate: date? +}; + +CREATE TYPE GleambookUserType AS { + id: int, + alias: string, + name: string, + userSince: datetime, + friendIds: {{ int }}, + employment: [EmploymentType] +}; + +CREATE DATASET GleambookUsers(GleambookUserType) PRIMARY KEY id; +CREATE DATASET ChirpUsers(ChirpUserType) PRIMARY KEY screenName; \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.2.update.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.2.update.sqlpp new file mode 100644 index 0000000..bcfcd32 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.2.update.sqlpp @@ -0,0 +1,43 @@ +/* + * 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. + */ + +// testing fix for ASTERIXDB-2289 related to field access with CASE +USE TinySocial; + +INSERT INTO ChirpUsers +([ +{"screenName":"NathanGiesen@211","lang":"en","friendsCount":18,"statusesCount":473,"name":"Nathan Giesen","followersCount":49416}, +{"screenName":"ColineGeyer@63","lang":"en","friendsCount":121,"statusesCount":362,"name":"Coline Geyer","followersCount":17159}, +{"screenName":"NilaMilliron_tw","lang":"en","friendsCount":445,"statusesCount":164,"name":"Nila Milliron","followersCount":22649}, +{"screenName":"ChangEwing_573","lang":"en","friendsCount":182,"statusesCount":394,"name":"Chang Ewing","followersCount":32136} +]); + +INSERT INTO GleambookUsers +([ +{"id":1,"alias":"Margarita","name":"MargaritaStoddard","nickname":"Mags","userSince":datetime("2012-08-20T10:10:00"),"friendIds":{{2,3,6,10}},"employment":[{"organizationName":"Codetechno","startDate":date("2006-08-06")},{"organizationName":"geomedia","startDate":date("2010-06-17"),"endDate":date("2010-01-26")}],"gender":"F"}, +{"id":2,"alias":"Isbel","name":"IsbelDull","nickname":"Izzy","userSince":datetime("2011-01-22T10:10:00"),"friendIds":{{1,4}},"employment":[{"organizationName":"Hexviafind","startDate":date("2010-04-27")}]}, +{"id":3,"alias":"Emory","name":"EmoryUnk","userSince":datetime("2012-07-10T10:10:00"),"friendIds":{{1,5,8,9}},"employment":[{"organizationName":"geomedia","startDate":date("2010-06-17"),"endDate":date("2010-01-26")}]}, +{"id":4,"alias":"Nicholas","name":"NicholasStroh","userSince":datetime("2010-12-27T10:10:00"),"friendIds":{{2}},"employment":[{"organizationName":"Zamcorporation","startDate":date("2010-06-08")}]}, +{"id":5,"alias":"Von","name":"VonKemble","userSince":datetime("2010-01-05T10:10:00"),"friendIds":{{3,6,10}},"employment":[{"organizationName":"Kongreen","startDate":date("2010-11-27")}]}, +{"id":6,"alias":"Willis","name":"WillisWynne","userSince":datetime("2005-01-17T10:10:00"),"friendIds":{{1,3,7}},"employment":[{"organizationName":"jaydax","startDate":date("2009-05-15")}]}, +{"id":7,"alias":"Suzanna","name":"SuzannaTillson","userSince":datetime("2012-08-07T10:10:00"),"friendIds":{{6}},"employment":[{"organizationName":"Labzatron","startDate":date("2011-04-19")}]}, +{"id":8,"alias":"Nila","name":"NilaMilliron","userSince":datetime("2008-01-01T10:10:00"),"friendIds":{{3}},"employment":[{"organizationName":"Plexlane","startDate":date("2010-02-28")}]}, +{"id":9,"alias":"Woodrow","name":"WoodrowNehling","nickname":"Woody","userSince":datetime("2005-09-20T10:10:00"),"friendIds":{{3,10}},"employment":[{"organizationName":"Zuncan","startDate":date("2003-04-22"),"endDate":date("2009-12-13")}]}, +{"id":10,"alias":"Bram","name":"BramHatch","userSince":datetime("2010-10-16T10:10:00"),"friendIds":{{1,5,9}},"employment":[{"organizationName":"physcane","startDate":date("2007-06-05"),"endDate":date("2011-11-05")}]} +]); \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.3.query.sqlpp new file mode 100644 index 0000000..d9a5644 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.3.query.sqlpp @@ -0,0 +1,25 @@ +/* + * 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. + */ + +// testing fix for ASTERIXDB-2289 related to field access with CASE +USE TinySocial; + +SELECT (CASE WHEN g.id = 6 THEN g ELSE u END).name as name +FROM ChirpUsers u JOIN GleambookUsers g ON u.name < g.name WHERE u.screenName = "ChangEwing_573" +ORDER BY name; \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.4.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.4.ddl.sqlpp new file mode 100644 index 0000000..da31d1d --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.4.ddl.sqlpp @@ -0,0 +1,21 @@ +/* + * 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. + */ + +// testing fix for ASTERIXDB-2289 related to field access with CASE +DROP DATAVERSE TinySocial IF EXISTS; \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.3.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.3.adm new file mode 100644 index 0000000..ec27645 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/field_access-ASTERIXDB-2289/field_access-ASTERIXDB-2289.3.adm @@ -0,0 +1,9 @@ +{ "name": "Chang Ewing" } +{ "name": "Chang Ewing" } +{ "name": "Chang Ewing" } +{ "name": "Chang Ewing" } +{ "name": "Chang Ewing" } +{ "name": "Chang Ewing" } +{ "name": "Chang Ewing" } +{ "name": "Chang Ewing" } +{ "name": "WillisWynne" } \ 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 45f86ec..9055c85 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml @@ -5625,6 +5625,11 @@ </compilation-unit> </test-case> <test-case FilePath="misc"> + <compilation-unit name="field_access-ASTERIXDB-2289"> + <output-dir compare="Text">field_access-ASTERIXDB-2289</output-dir> + </compilation-unit> + </test-case> + <test-case FilePath="misc"> <compilation-unit name="comp-ASTERIXDB-2415"> <output-dir compare="Text">query-ASTERIXDB-1671</output-dir> </compilation-unit> diff --git a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorPropertiesUtil.java b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorPropertiesUtil.java index f43d367..49d4748 100644 --- a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorPropertiesUtil.java +++ b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorPropertiesUtil.java @@ -311,7 +311,7 @@ // Can't move nonPures! AssignOperator assign = (AssignOperator) op; for (Mutable<ILogicalExpression> expr : assign.getExpressions()) { - if (containsNonpureCall(expr.getValue())) { + if (!expr.getValue().isFunctional()) { return false; } } @@ -321,22 +321,6 @@ return false; } return true; - } - - private static boolean containsNonpureCall(ILogicalExpression expr) { - if (expr.getExpressionTag() == LogicalExpressionTag.FUNCTION_CALL) { - AbstractFunctionCallExpression fExpr = (AbstractFunctionCallExpression) expr; - if (!fExpr.getFunctionInfo().isFunctional()) { - return true; - } - for (Mutable<ILogicalExpression> subExpr : fExpr.getArguments()) { - if (containsNonpureCall(subExpr.getValue())) { - return true; - } - } - - } - return false; } /** -- To view, visit https://asterix-gerrit.ics.uci.edu/3399 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I911e4e9018c15e8f226e46fa610d222eb2301fcd Gerrit-Change-Number: 3399 Gerrit-PatchSet: 5 Gerrit-Owner: Ali Alsuliman <ali.al.solai...@gmail.com> Gerrit-Reviewer: Ali Alsuliman <ali.al.solai...@gmail.com> Gerrit-Reviewer: Anon. E. Moose (1000171) Gerrit-Reviewer: Dmitry Lychagin <dmitry.lycha...@couchbase.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Michael Blow <mb...@apache.org> Gerrit-Reviewer: Till Westmann <ti...@apache.org>
