[ASTERIXDB-3205][COMP] Avoid inlining non-pure functions in aggregate functions
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
Currently, the compiler blindly inlines all functions
from assign operators into aggregate functions. However,
non-pure functions should not be inlined.
Change-Id: Ib3fbd684f1732a276d6033bf38507df41d33b843
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17589
Reviewed-by: Wail Alkowaileet <wael.y.k@gmail.com>
Reviewed-by: Ali Alsuliman <ali.al.solaiman@gmail.com>
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.00.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.00.ddl.sqlpp
new file mode 100644
index 0000000..99898f9
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.00.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 OpenType AS {
+ id: int
+};
+
+CREATE DATASET MyDataset(OpenType)
+PRIMARY KEY id;
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.01.update.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.01.update.sqlpp
new file mode 100644
index 0000000..751aaba
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.01.update.sqlpp
@@ -0,0 +1,24 @@
+/*
+ * 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 MyDataset(
+ SELECT x id, x num
+ FROM range(1, 10) x
+);
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.02.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.02.query.sqlpp
new file mode 100644
index 0000000..116c61a
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.02.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 : Do not inline random(1) in COUNT()
+* Date : Jun 2nd 2023
+*/
+USE test;
+SELECT COUNT(random(1)) randCount
+FROM MyDataset md
+GROUP BY 1
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.03.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.03.query.sqlpp
new file mode 100644
index 0000000..1e02ae2
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.03.query.sqlpp
@@ -0,0 +1,27 @@
+/*
+ * 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 : Do not inline random(1) in COUNT()
+* Date : Jun 2nd 2023
+*/
+USE test;
+
+SELECT COUNT(random(1)) randCount, COUNT(md.num + 1) numCount
+FROM MyDataset md
+GROUP BY 1
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.04.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.04.query.sqlpp
new file mode 100644
index 0000000..c583723
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.04.query.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 : Do not inline random(1) in COUNT()
+* Date : Jun 2nd 2023
+* Notes : We can inline COUNT(md.num + 1) if ASTERIXDB-3206 is fixed
+*/
+USE test;
+
+SELECT COUNT(md.num + 1) numCount, COUNT(random(1)) randCount
+FROM MyDataset md
+GROUP BY 1
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.05.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.05.query.sqlpp
new file mode 100644
index 0000000..715b73f
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.05.query.sqlpp
@@ -0,0 +1,27 @@
+/*
+ * 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 : Do not inline random(1) in COUNT()
+* Date : Jun 2nd 2023
+*/
+USE test;
+
+SELECT COUNT(random(1) + round(md.num / 3)) randCount, COUNT(md.num + 1) numCount
+FROM MyDataset md
+GROUP BY 1
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.06.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.06.query.sqlpp
new file mode 100644
index 0000000..20e2c78
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/aggregate/non-pure-function/non-pure-function.06.query.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 : Do not inline random(1) in COUNT()
+* Date : Jun 2nd 2023
+* Notes : We can inline COUNT(md.num + 1) if ASTERIXDB-3206 is fixed
+*/
+USE test;
+
+SELECT COUNT(md.num + 1) numCount, COUNT(random(1) + round(md.num / 3)) randCount
+FROM MyDataset md
+GROUP BY 1
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/non-pure-function/non-pure-function.02.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/non-pure-function/non-pure-function.02.adm
new file mode 100644
index 0000000..6390f8a
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/non-pure-function/non-pure-function.02.adm
@@ -0,0 +1 @@
+{ "randCount": 10 }
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/non-pure-function/non-pure-function.03.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/non-pure-function/non-pure-function.03.adm
new file mode 100644
index 0000000..160683e
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/non-pure-function/non-pure-function.03.adm
@@ -0,0 +1 @@
+{ "randCount": 10, "numCount": 10 }
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/non-pure-function/non-pure-function.04.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/non-pure-function/non-pure-function.04.adm
new file mode 100644
index 0000000..a3724b3
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/non-pure-function/non-pure-function.04.adm
@@ -0,0 +1 @@
+{ "numCount": 10, "randCount": 10 }
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/non-pure-function/non-pure-function.05.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/non-pure-function/non-pure-function.05.adm
new file mode 100644
index 0000000..160683e
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/non-pure-function/non-pure-function.05.adm
@@ -0,0 +1 @@
+{ "randCount": 10, "numCount": 10 }
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/non-pure-function/non-pure-function.06.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/non-pure-function/non-pure-function.06.adm
new file mode 100644
index 0000000..a3724b3
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/non-pure-function/non-pure-function.06.adm
@@ -0,0 +1 @@
+{ "numCount": 10, "randCount": 10 }
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 c0784c1..41c0424 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
@@ -1612,6 +1612,11 @@
<output-dir compare="Text">min_max_arrays</output-dir>
</compilation-unit>
</test-case>
+ <test-case FilePath="aggregate">
+ <compilation-unit name="non-pure-function">
+ <output-dir compare="Text">non-pure-function</output-dir>
+ </compilation-unit>
+ </test-case>
</test-group>
<test-group name="aggregate-sql">
<test-case FilePath="aggregate-sql">
diff --git a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/InlineAssignIntoAggregateRule.java b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/InlineAssignIntoAggregateRule.java
index d966ed1..ad7703d 100644
--- a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/InlineAssignIntoAggregateRule.java
+++ b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/InlineAssignIntoAggregateRule.java
@@ -84,11 +84,7 @@
}
AssignOperator assignOp = (AssignOperator) op2;
VarExprSubstitution ves = new VarExprSubstitution(assignOp.getVariables(), assignOp.getExpressions());
- inlineVariables(aggOp, ves);
- List<Mutable<ILogicalOperator>> op1InpList = aggOp.getInputs();
- op1InpList.clear();
- op1InpList.add(op2.getInputs().get(0));
- return true;
+ return inlineVariables(aggOp, ves);
}
private boolean inlineOuterInputAssignIntoAgg(AggregateOperator aggOp,
@@ -107,18 +103,20 @@
for (Mutable<ILogicalExpression> exprRef : aggOp.getExpressions()) {
ILogicalExpression expr = exprRef.getValue();
Pair<Boolean, ILogicalExpression> p = expr.accept(ves, null);
- if (p.first) {
- exprRef.setValue(p.second);
+ ILogicalExpression originalExpr = p.second;
+ if (p.first & originalExpr.isFunctional()) {
+ exprRef.setValue(originalExpr);
inlined = true;
}
}
return inlined;
}
- private class VarExprSubstitution extends AbstractConstVarFunVisitor<Pair<Boolean, ILogicalExpression>, Void> {
+ private static class VarExprSubstitution
+ extends AbstractConstVarFunVisitor<Pair<Boolean, ILogicalExpression>, Void> {
- private List<LogicalVariable> variables;
- private List<Mutable<ILogicalExpression>> expressions;
+ private final List<LogicalVariable> variables;
+ private final List<Mutable<ILogicalExpression>> expressions;
public VarExprSubstitution(List<LogicalVariable> variables, List<Mutable<ILogicalExpression>> expressions) {
this.variables = variables;
@@ -127,7 +125,7 @@
@Override
public Pair<Boolean, ILogicalExpression> visitConstantExpression(ConstantExpression expr, Void arg) {
- return new Pair<Boolean, ILogicalExpression>(false, expr);
+ return new Pair<>(false, expr);
}
@Override
@@ -137,12 +135,13 @@
for (Mutable<ILogicalExpression> eRef : expr.getArguments()) {
ILogicalExpression e = eRef.getValue();
Pair<Boolean, ILogicalExpression> p = e.accept(this, arg);
- if (p.first) {
- eRef.setValue(p.second.cloneExpression());
+ ILogicalExpression originalExpr = p.second;
+ if (p.first & originalExpr.isFunctional()) {
+ eRef.setValue(originalExpr.cloneExpression());
changed = true;
}
}
- return new Pair<Boolean, ILogicalExpression>(changed, expr);
+ return new Pair<>(changed, expr);
}
@Override
@@ -151,9 +150,9 @@
LogicalVariable v = expr.getVariableReference();
int idx = variables.indexOf(v);
if (idx < 0) {
- return new Pair<Boolean, ILogicalExpression>(false, expr);
+ return new Pair<>(false, expr);
} else {
- return new Pair<Boolean, ILogicalExpression>(true, expressions.get(idx).getValue());
+ return new Pair<>(true, expressions.get(idx).getValue());
}
}