Yingyi Bu has submitted this change and it was merged. Change subject: [ASTERIXDB-1897][COMP][RT] Fix complex group-by. ......................................................................
[ASTERIXDB-1897][COMP][RT] Fix complex group-by. - user model changes: no - storage format changes: no - interface changes: no Details: - Fix type computer for numeric aggregations; - Fix error reporting for SubplanRuntimeFactory; - Add a negative test query. Change-Id: Iebb393820a8edd0c54d80248b2a33c77d4f6fd7b Reviewed-on: https://asterix-gerrit.ics.uci.edu/1923 Sonar-Qube: Jenkins <[email protected]> Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> Reviewed-by: Till Westmann <[email protected]> --- A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/policy-05/policy-05.1.ddl.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/policy-05/policy-05.2.update.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/policy-05/policy-05.3.query.sqlpp M asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml M asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/NumericAggTypeComputer.java M asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/TypeComputeUtils.java M hyracks-fullstack/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/meta/SubplanRuntimeFactory.java 7 files changed, 233 insertions(+), 25 deletions(-) Approvals: Till Westmann: Looks good to me, approved Jenkins: Verified; No violations found; Verified Objections: Jenkins: Violations found diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/policy-05/policy-05.1.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/policy-05/policy-05.1.ddl.sqlpp new file mode 100644 index 0000000..0242f1c --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/policy-05/policy-05.1.ddl.sqlpp @@ -0,0 +1,29 @@ +/* + * 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 gby IF EXISTS; +CREATE DATAVERSE gby; + +USE gby; + +CREATE TYPE PolicyType AS { + id: UUID +} + +CREATE DATASET policies(PolicyType) PRIMARY KEY id AUTOGENERATED; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/policy-05/policy-05.2.update.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/policy-05/policy-05.2.update.sqlpp new file mode 100644 index 0000000..3b865bb --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/policy-05/policy-05.2.update.sqlpp @@ -0,0 +1,137 @@ +/* + * 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 gby; + +INSERT INTO policies +( + [ { + "policyno": "C123", + "state": "CA", + "zipcode": "96008", + "make": "Honda", + "accidents": [ + { + "year": "2015", + "cost": 5000 + }, + { + "year": "2016", + "cost": 8000 + }, + { + "year": "2016", + "cost": 6000 + } + ] + }, + { + "policyno": "C124", + "state": "CA", + "zipcode": "96853", + "make": "Ford", + "accidents": [ + { + "year": "2015", + "cost": 5000 + }, + { + "year": "2015", + "cost": 8000 + }, + { + "year": "2016", + "cost": 6000 + } + ] + }, + { + "policyno": "A123", + "state": "AZ", + "zipcode": "86008", + "make": "Honda", + "accidents": [ + { + "year": "2015", + "cost": 5000 + }, + { + "year": "2016", + "cost": 8000 + }, + { + "year": "2016", + "cost": 6000 + } + ] + }, + { + "policyno": "A124", + "state": "AZ", + "zipcode": "86853", + "make": "Ford", + "accidents": [ + { + "year": "2015", + "cost": 5000 + }, + { + "year": "2016", + "cost": 8000 + }, + { + "year": "2016", + "cost": 6000 + } + ] + }, + { + "policyno": "U123", + "state": "UT", + "zipcode": "66008", + "make": "Honda", + "accidents": [ + { + "year": "2015", + "cost": 5000 + }, + { + "year": "2016", + "cost": 8000 + }, + { + "year": "2016", + "cost": 6000 + } + ] + }, + { + "policyno": "U124", + "state": "UT", + "zipcode": "66853", + "make": "Ford", + "accidents": [ ] + }, + { + "policyno": "U125", + "state": "UT", + "zipcode": "66853", + "make": "Ford" + } ] +); \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/policy-05/policy-05.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/policy-05/policy-05.3.query.sqlpp new file mode 100644 index 0000000..9a03c50 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/policy-05/policy-05.3.query.sqlpp @@ -0,0 +1,36 @@ +/* + * 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 gby; + +FROM policies p +GROUP BY state GROUP AS g +SELECT state, + ( + FROM g + SELECT VALUE SUM( + ( + FROM g.p.accidents a + WHERE a.year = "2016" + SELECT COUNT(*) + )[0] + ) + )[0] / (COUNT(*) * 1.0 ) AS risk +ORDER BY risk DESC +LIMIT 5; 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 47c0529..70b3486 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml @@ -2881,6 +2881,12 @@ <output-dir compare="Text">policy-04</output-dir> </compilation-unit> </test-case> + <test-case FilePath="group-by"> + <compilation-unit name="policy-05"> + <output-dir compare="Text">none</output-dir> + <expected-error>Invalid item type: function agg-sum cannot process item type object in an input array (or multiset)</expected-error> + </compilation-unit> + </test-case> </test-group> <test-group name="index-join"> <test-case FilePath="index-join"> diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/NumericAggTypeComputer.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/NumericAggTypeComputer.java index f2ae015..ec9c59f 100644 --- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/NumericAggTypeComputer.java +++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/NumericAggTypeComputer.java @@ -22,6 +22,7 @@ import org.apache.asterix.om.typecomputer.base.AbstractResultTypeComputer; import org.apache.asterix.om.types.ATypeTag; import org.apache.asterix.om.types.AUnionType; +import org.apache.asterix.om.types.BuiltinType; import org.apache.asterix.om.types.IAType; import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException; import org.apache.hyracks.algebricks.core.algebra.base.ILogicalExpression; @@ -52,7 +53,6 @@ @Override protected IAType getResultType(ILogicalExpression expr, IAType... strippedInputTypes) throws AlgebricksException { ATypeTag tag = strippedInputTypes[0].getTypeTag(); - IAType type = null; switch (tag) { case DOUBLE: case FLOAT: @@ -61,11 +61,12 @@ case SMALLINT: case TINYINT: case ANY: - type = strippedInputTypes[0]; - break; + IAType type = strippedInputTypes[0]; + return AUnionType.createNullableType(type, "AggResult"); default: - break; + // All other possible cases. + return BuiltinType.ANULL; } - return AUnionType.createNullableType(type, "AggResult"); + } } diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/TypeComputeUtils.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/TypeComputeUtils.java index a1bbb86..d6825e3 100644 --- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/TypeComputeUtils.java +++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/TypeComputeUtils.java @@ -22,7 +22,6 @@ import org.apache.asterix.om.types.ARecordType; import org.apache.asterix.om.types.ATypeTag; import org.apache.asterix.om.types.AUnionType; -import org.apache.asterix.om.types.AUnorderedListType; import org.apache.asterix.om.types.BuiltinType; import org.apache.asterix.om.types.IAType; import org.apache.asterix.om.utils.RecordUtil; @@ -100,6 +99,21 @@ return BuiltinType.AMISSING; } if (category == NULL) { + return BuiltinType.ANULL; + } + boolean metNull = false; + for (IAType knownInputType : knownInputTypes) { + ATypeTag typeTag = knownInputType.getTypeTag(); + // Returns missing if there is one missing. + if (typeTag == ATypeTag.MISSING) { + return knownInputType; + } + if (typeTag == ATypeTag.NULL) { + metNull = true; + } + } + // Returns null if there is one null but there is no missing. + if (metNull) { return BuiltinType.ANULL; } return TypeComputeUtils.getResultType(resultTypeGenerator.getResultType(expr, knownInputTypes), category); @@ -215,21 +229,6 @@ return (AOrderedListType) innerType; } } - return null; - } - - public static AUnorderedListType extractUnorderedListType(IAType t) { - if (t.getTypeTag() == ATypeTag.MULTISET) { - return (AUnorderedListType) t; - } - if (t.getTypeTag() == ATypeTag.UNION) { - AUnionType unionType = (AUnionType) t; - IAType innerType = unionType.getActualType(); - if (innerType.getTypeTag() == ATypeTag.MULTISET) { - return (AUnorderedListType) innerType; - } - } - return null; } diff --git a/hyracks-fullstack/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/meta/SubplanRuntimeFactory.java b/hyracks-fullstack/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/meta/SubplanRuntimeFactory.java index 8b13e09..f6a349f 100644 --- a/hyracks-fullstack/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/meta/SubplanRuntimeFactory.java +++ b/hyracks-fullstack/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/meta/SubplanRuntimeFactory.java @@ -91,7 +91,8 @@ private boolean smthWasWritten = false; private FrameTupleAccessor ta = new FrameTupleAccessor( pipeline.getRecordDescriptors()[pipeline.getRecordDescriptors().length - 1]); - private ArrayTupleBuilder tb = new ArrayTupleBuilder(nullWriters.length); + private ArrayTupleBuilder tb = new ArrayTupleBuilder( + nullWriters.length + SubplanRuntimeFactory.this.inputRecordDesc.getFieldCount()); @Override public void open() throws HyracksDataException { @@ -110,17 +111,16 @@ @Override public void close() throws HyracksDataException { - if (!smthWasWritten) { + if (!smthWasWritten && !failed) { // the case when we need to write nulls appendNullsToTuple(); appendToFrameFromTupleBuilder(tb); } - } @Override public void fail() throws HyracksDataException { - writer.fail(); + // writer.fail() is called by the outer class' writer.fail(). } private void appendNullsToTuple() throws HyracksDataException { -- To view, visit https://asterix-gerrit.ics.uci.edu/1923 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iebb393820a8edd0c54d80248b2a33c77d4f6fd7b Gerrit-PatchSet: 5 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Yingyi Bu <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]>
