Dmitry Lychagin has uploaded a new change for review.
https://asterix-gerrit.ics.uci.edu/2821
Change subject: [ASTERIXDB-2428][COMP] Incorrect result with limit if offset is
negative
......................................................................
[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
---
M
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java
A
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit_negative_value/limit_negative_value.1.ddl.sqlpp
A
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit_negative_value/limit_negative_value.2.update.sqlpp
A
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit_negative_value/limit_negative_value.3.query.sqlpp
A
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit_negative_value/limit_negative_value.4.query.sqlpp
A
asterixdb/asterix-app/src/test/resources/runtimets/results/limit/limit_negative_value/limit_negative_value.3.adm
A
asterixdb/asterix-app/src/test/resources/runtimets/results/limit/limit_negative_value/limit_negative_value.4.adm
M asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
8 files changed, 203 insertions(+), 7 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb
refs/changes/21/2821/1
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..96432e3 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);
@@ -1370,6 +1369,37 @@
opLim.setSourceLocation(sourceLoc);
}
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 valueExpr;
}
private static AbstractFunctionCallExpression
createFunctionCallExpressionForBuiltinOperator(OperatorType t,
@@ -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>
--
To view, visit https://asterix-gerrit.ics.uci.edu/2821
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2238cc5d8f48e14aa2b74d120248a4848dd35691
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Dmitry Lychagin <[email protected]>
