>From Dmitry Lychagin <[email protected]>: Dmitry Lychagin has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/15305 )
Change subject: [ASTERIXDB-3015][FUN] Fix avg() handling of first non-number ...................................................................... [ASTERIXDB-3015][FUN] Fix avg() handling of first non-number - user model changes: no - storage format changes: no - interface changes: no Details: - Fix incorrect result produced by avg() function when its first input value is not a number Change-Id: I21cc52c56fb79a609aabdf994a7b6e19d570b6ea Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/15305 Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> Reviewed-by: Dmitry Lychagin <[email protected]> Reviewed-by: Ali Alsuliman <[email protected]> --- A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.1.ddl.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.2.update.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.3.query.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.4.query.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.5.query.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate-sql-sugar/avg_mixed/avg_mixed.3.adm A asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate-sql-sugar/avg_mixed/avg_mixed.4.adm A asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate-sql-sugar/avg_mixed/avg_mixed.5.adm M asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml M asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/std/AbstractAvgAggregateFunction.java 10 files changed, 167 insertions(+), 5 deletions(-) Approvals: Dmitry Lychagin: Looks good to me, but someone else must approve Ali Alsuliman: Looks good to me, approved Jenkins: Verified; Verified Objections: Anon. E. Moose #1000171: Violations found diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.1.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.1.ddl.sqlpp new file mode 100644 index 0000000..b09662f --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.1.ddl.sqlpp @@ -0,0 +1,28 @@ +/* + * 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. + */ +/* + * Description : Run avg over a list of mixed types (ASTERIXDB-3015) + */ + +drop dataverse test if exists; +create dataverse test; + +use test; + +create dataset d1(id bigint) open type primary key id; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.2.update.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.2.update.sqlpp new file mode 100644 index 0000000..a5943c9 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.2.update.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. + */ +/* + * Description : Run avg over a list of mixed types (ASTERIXDB-3015) + */ + +use test; + +insert into d1 ( + [ + { "id": 1, "i": 1, "v": [ "a", 1, 2, 3 ] }, + { "id": 2, "i": 2, "v": [ "b", 4, 5, 6 ] } + ] +); \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.3.query.sqlpp new file mode 100644 index 0000000..eff95e0 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.3.query.sqlpp @@ -0,0 +1,26 @@ +/* + * 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. + */ +/* +* Description : Run avg over a list of mixed types (ASTERIXDB-3015) +* Expected Res : Failure +*/ + +-- param max-warnings:json=100 + +select avg(x) a from ["a", 1, 2, 3] x; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.4.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.4.query.sqlpp new file mode 100644 index 0000000..756194b --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.4.query.sqlpp @@ -0,0 +1,31 @@ +/* + * 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. + */ +/* +* Description : Run avg over a list of mixed types (ASTERIXDB-3015) +* Expected Res : Failure +*/ + +-- param max-warnings:json=100 + +use test; + +select d1.id, avg(v) a +from d1, d1.v +group by d1.id +order by d1.id; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.5.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.5.query.sqlpp new file mode 100644 index 0000000..71b22e7 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate-sql-sugar/avg_mixed/avg_mixed.5.query.sqlpp @@ -0,0 +1,32 @@ +/* + * 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. + */ +/* +* Description : Run avg over a list of mixed types (ASTERIXDB-3015) +* Expected Res : Failure +*/ + +-- param max-warnings:json=100 + +use test; + + +select d1.id, avg(v) a +from d1, d1.v +/* +hash */ group by d1.id +order by d1.id; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate-sql-sugar/avg_mixed/avg_mixed.3.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate-sql-sugar/avg_mixed/avg_mixed.3.adm new file mode 100644 index 0000000..53bdaa0 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate-sql-sugar/avg_mixed/avg_mixed.3.adm @@ -0,0 +1 @@ +{ "a": 2.0 } \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate-sql-sugar/avg_mixed/avg_mixed.4.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate-sql-sugar/avg_mixed/avg_mixed.4.adm new file mode 100644 index 0000000..35fa557 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate-sql-sugar/avg_mixed/avg_mixed.4.adm @@ -0,0 +1,2 @@ +{ "id": 1, "a": 2.0 } +{ "id": 2, "a": 5.0 } \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate-sql-sugar/avg_mixed/avg_mixed.5.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate-sql-sugar/avg_mixed/avg_mixed.5.adm new file mode 100644 index 0000000..35fa557 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate-sql-sugar/avg_mixed/avg_mixed.5.adm @@ -0,0 +1,2 @@ +{ "id": 1, "a": 2.0 } +{ "id": 2, "a": 5.0 } \ 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 bc9353b..8b6b7c0 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml @@ -2883,6 +2883,14 @@ <expected-error>ASX1079: Compilation error: arrayagg is a SQL-92 aggregate function. The SQL++ core aggregate function strict_arrayagg could potentially express the intent.</expected-error> </compilation-unit> </test-case> + <test-case FilePath="aggregate-sql-sugar" check-warnings="true"> + <compilation-unit name="avg_mixed"> + <output-dir compare="Text">avg_mixed</output-dir> + <expected-warn>ASX0004: Unsupported type: agg-avg cannot process input type string (in line 26, at column 12)</expected-warn> + <expected-warn>ASX0004: Unsupported type: agg-avg cannot process input type string (in line 28, at column 19)</expected-warn> + <expected-warn>ASX0004: Unsupported type: agg-avg cannot process input type string (in line 29, at column 19)</expected-warn> + </compilation-unit> + </test-case> <test-case FilePath="aggregate-sql-sugar"> <compilation-unit name="distinct_mixed"> <output-dir compare="Text">distinct_mixed</output-dir> diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/std/AbstractAvgAggregateFunction.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/std/AbstractAvgAggregateFunction.java index d2367f9..fc668d2 100644 --- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/std/AbstractAvgAggregateFunction.java +++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/std/AbstractAvgAggregateFunction.java @@ -136,12 +136,13 @@ int offset = inputVal.getStartOffset(); ATypeTag typeTag = EnumDeserializer.ATYPETAGDESERIALIZER.deserialize(data[offset]); + ATypeTag aggTypeTag = aggType; if (typeTag == ATypeTag.MISSING || typeTag == ATypeTag.NULL) { processNull(); return; - } else if (aggType == ATypeTag.SYSTEM_NULL) { - aggType = typeTag; - } else if (typeTag != ATypeTag.SYSTEM_NULL && !ATypeHierarchy.isCompatible(typeTag, aggType)) { + } else if (aggTypeTag == ATypeTag.SYSTEM_NULL) { + aggTypeTag = typeTag; + } else if (typeTag != ATypeTag.SYSTEM_NULL && !ATypeHierarchy.isCompatible(typeTag, aggTypeTag)) { // Issue warning only once and treat current tuple as null if (!isWarned) { isWarned = true; @@ -149,8 +150,8 @@ } processNull(); return; - } else if (ATypeHierarchy.canPromote(aggType, typeTag)) { - aggType = typeTag; + } else if (ATypeHierarchy.canPromote(aggTypeTag, typeTag)) { + aggTypeTag = typeTag; } switch (typeTag) { @@ -195,6 +196,7 @@ } } count++; + aggType = aggTypeTag; } protected void finishPartialResults(IPointable result) throws HyracksDataException { -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/15305 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: I21cc52c56fb79a609aabdf994a7b6e19d570b6ea Gerrit-Change-Number: 15305 Gerrit-PatchSet: 3 Gerrit-Owner: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-MessageType: merged
