[ASTERIXDB-2428][COMP] Incorrect result with limit if offset is negative
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
- Guarantee non negative limit and offset value during plan generation,
so it is correct to add them in CopyLimitDownRule
Change-Id: I2238cc5d8f48e14aa2b74d120248a4848dd35691
Reviewed-on: https://asterix-gerrit.ics.uci.edu/2821
Sonar-Qube: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Contrib: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Taewoo Kim <wangsaeu@gmail.com>
diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java
index 5410a94..aa4fb75 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java
@@ -84,8 +84,10 @@
import org.apache.asterix.metadata.functions.ExternalFunctionCompilerUtil;
import org.apache.asterix.metadata.utils.DatasetUtil;
import org.apache.asterix.om.base.ABoolean;
+import org.apache.asterix.om.base.AInt32;
import org.apache.asterix.om.base.AInt64;
import org.apache.asterix.om.base.AString;
+import org.apache.asterix.om.base.IAObject;
import org.apache.asterix.om.constants.AsterixConstantValue;
import org.apache.asterix.om.functions.BuiltinFunctions;
import org.apache.asterix.om.functions.FunctionInfo;
@@ -1351,16 +1353,13 @@
LimitOperator opLim;
Pair<ILogicalExpression, Mutable<ILogicalOperator>> p1 = langExprToAlgExpression(lc.getLimitExpr(), tupSource);
- AbstractFunctionCallExpression maxObjectsExpr =
- createFunctionCallExpression(BuiltinFunctions.TREAT_AS_INTEGER, lc.getLimitExpr().getSourceLocation());
- maxObjectsExpr.getArguments().add(new MutableObject<>(p1.first));
-
+ ILogicalExpression maxObjectsExpr =
+ createLimitOffsetValueExpression(p1.first, lc.getLimitExpr().getSourceLocation());
Expression offset = lc.getOffset();
if (offset != null) {
Pair<ILogicalExpression, Mutable<ILogicalOperator>> p2 = langExprToAlgExpression(offset, p1.second);
- AbstractFunctionCallExpression offsetExpr =
- createFunctionCallExpression(BuiltinFunctions.TREAT_AS_INTEGER, lc.getOffset().getSourceLocation());
- offsetExpr.getArguments().add(new MutableObject<>(p2.first));
+ ILogicalExpression offsetExpr =
+ createLimitOffsetValueExpression(p2.first, lc.getOffset().getSourceLocation());
opLim = new LimitOperator(maxObjectsExpr, offsetExpr);
opLim.getInputs().add(p2.second);
opLim.setSourceLocation(sourceLoc);
@@ -1372,6 +1371,37 @@
return new Pair<>(opLim, null);
}
+ private ILogicalExpression createLimitOffsetValueExpression(ILogicalExpression inputExpr, SourceLocation sourceLoc)
+ throws CompilationException {
+ // generates expression for limit and offset value:
+ //
+ // switch-case(treat-as-integer(user_value_expr) > 0, true, treat-as-integer(user_value_expr), 0)
+ //
+ // this guarantees that the value is always an integer and greater or equals to 0,
+ // so CopyLimitDownRule works correctly when computing the total limit,
+ // and other rules which assume integer type
+
+ AInt32 zero = new AInt32(0);
+
+ AbstractFunctionCallExpression valueExpr =
+ createFunctionCallExpression(BuiltinFunctions.TREAT_AS_INTEGER, sourceLoc);
+ valueExpr.getArguments().add(new MutableObject<>(inputExpr));
+
+ AbstractFunctionCallExpression cmpExpr =
+ createFunctionCallExpressionForBuiltinOperator(OperatorType.GT, sourceLoc);
+ cmpExpr.getArguments().add(new MutableObject<>(valueExpr));
+ cmpExpr.getArguments().add(new MutableObject<>(createConstantExpression(zero, sourceLoc)));
+
+ AbstractFunctionCallExpression switchExpr =
+ createFunctionCallExpression(BuiltinFunctions.SWITCH_CASE, sourceLoc);
+ switchExpr.getArguments().add(new MutableObject<>(cmpExpr));
+ switchExpr.getArguments().add(new MutableObject<>(createConstantExpression(ABoolean.TRUE, sourceLoc)));
+ switchExpr.getArguments().add(new MutableObject<>(valueExpr.cloneExpression()));
+ switchExpr.getArguments().add(new MutableObject<>(createConstantExpression(zero, sourceLoc)));
+
+ return switchExpr;
+ }
+
private static AbstractFunctionCallExpression createFunctionCallExpressionForBuiltinOperator(OperatorType t,
SourceLocation sourceLoc) throws CompilationException {
FunctionIdentifier fid;
@@ -1878,4 +1908,10 @@
}
return new Pair<>(topUnionAllOp, topUnionVar);
}
+
+ private ConstantExpression createConstantExpression(IAObject value, SourceLocation sourceLoc) {
+ ConstantExpression constExpr = new ConstantExpression(new AsterixConstantValue(value));
+ constExpr.setSourceLocation(sourceLoc);
+ return constExpr;
+ }
}
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit_negative_value/limit_negative_value.1.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit_negative_value/limit_negative_value.1.ddl.sqlpp
new file mode 100644
index 0000000..33a9c58
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit_negative_value/limit_negative_value.1.ddl.sqlpp
@@ -0,0 +1,39 @@
+/*
+ * 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 : Test push down limit into the primary index scan operator
+ * Expected Result : Success
+ */
+
+drop dataverse test if exists;
+create dataverse test;
+
+use test;
+
+create type test.DBLPType as
+{
+ id : bigint,
+ dblpid : string,
+ title : string,
+ authors : string,
+ misc : string
+};
+
+create dataset DBLP1(DBLPType) primary key id;
+
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit_negative_value/limit_negative_value.2.update.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit_negative_value/limit_negative_value.2.update.sqlpp
new file mode 100644
index 0000000..6be85ee
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit_negative_value/limit_negative_value.2.update.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.
+ */
+use test;
+
+load dataset DBLP1 using localfs ((`path`=`asterix_nc1://data/dblp-small/dblp-small-id.txt`),(`format`=`delimited-text`),(`delimiter`=`:`));
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit_negative_value/limit_negative_value.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit_negative_value/limit_negative_value.3.query.sqlpp
new file mode 100644
index 0000000..194ad19
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit_negative_value/limit_negative_value.3.query.sqlpp
@@ -0,0 +1,67 @@
+/*
+ * 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 : Test negative limit and offset values
+ * Expected Result : Success
+ */
+
+{
+ "t1": array_count((
+ select value t
+ from [6,5,4,3,2,1] t
+ order by t
+ limit -2
+ )),
+
+ "t2": array_count((
+ select value t
+ from [6,5,4,3,2,1] t
+ order by t
+ limit -get_year(current_date())
+ )),
+
+ "t3": (
+ select value t
+ from [6,5,4,3,2,1] t
+ order by t
+ limit 2 offset -4
+ ),
+
+ "t4": (
+ select value t
+ from [6,5,4,3,2,1] t
+ order by t
+ limit 2 offset -get_year(current_date())
+ ),
+
+ "t5": array_count((
+ select value t
+ from [6,5,4,3,2,1] t
+ order by t
+ limit -2 offset -2
+ )),
+
+ "t6": array_count((
+ select value t
+ from [6,5,4,3,2,1] t
+ order by t
+ limit -get_year(current_date()) offset -get_year(current_datetime())
+ ))
+}
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit_negative_value/limit_negative_value.4.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit_negative_value/limit_negative_value.4.query.sqlpp
new file mode 100644
index 0000000..df89aca
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit_negative_value/limit_negative_value.4.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.
+ */
+
+use test;
+
+select value paper
+from DBLP1 as paper
+order by dblpid
+limit 2 offset -get_year(current_date());
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/limit/limit_negative_value/limit_negative_value.3.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/limit/limit_negative_value/limit_negative_value.3.adm
new file mode 100644
index 0000000..af2cdfd
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/limit/limit_negative_value/limit_negative_value.3.adm
@@ -0,0 +1 @@
+{ "t1": 0, "t2": 0, "t3": [ 1, 2 ], "t4": [ 1, 2 ], "t5": 0, "t6": 0 }
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/limit/limit_negative_value/limit_negative_value.4.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/limit/limit_negative_value/limit_negative_value.4.adm
new file mode 100644
index 0000000..6e7b664
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/limit/limit_negative_value/limit_negative_value.4.adm
@@ -0,0 +1,2 @@
+{ "id": 34, "dblpid": "books/acm/Kim95", "title": "Modern Database Systems The Object Model, Interoperability, and Beyond.", "authors": "", "misc": "2004-03-08 Won Kim Modern Database Systems ACM Press and Addison-Wesley 1995 0-201-59098-0 db/books/collections/kim95.html" }
+{ "id": 1, "dblpid": "books/acm/kim95/AnnevelinkACFHK95", "title": "Object SQL - A Language for the Design and Implementation of Object Databases.", "authors": "Jurgen Annevelink Rafiul Ahad Amelia Carlson Daniel H. Fishman Michael L. Heytens William Kent", "misc": "2002-01-03 42-68 1995 Modern Database Systems db/books/collections/kim95.html#AnnevelinkACFHK95" }
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 49bed4b..ada210e 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
@@ -10161,6 +10161,11 @@
</test-group>
<test-group name="limit">
<test-case FilePath="limit">
+ <compilation-unit name="limit_negative_value">
+ <output-dir compare="Text">limit_negative_value</output-dir>
+ </compilation-unit>
+ </test-case>
+ <test-case FilePath="limit">
<compilation-unit name="limit_type_01">
<output-dir compare="Text">limit_type_01</output-dir>
</compilation-unit>