You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by al...@apache.org on 2022/12/12 15:02:42 UTC
[ignite] branch master updated: IGNITE-18363 SQL Calcite: Fix row count estimate by limit with offset - Fixes #10428.
This is an automated email from the ASF dual-hosted git repository.
alexpl pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/master by this push:
new fe445101265 IGNITE-18363 SQL Calcite: Fix row count estimate by limit with offset - Fixes #10428.
fe445101265 is described below
commit fe445101265968109c5c4de82e635ec1700f58c4
Author: Aleksey Plekhanov <pl...@gmail.com>
AuthorDate: Mon Dec 12 18:00:28 2022 +0300
IGNITE-18363 SQL Calcite: Fix row count estimate by limit with offset - Fixes #10428.
Signed-off-by: Aleksey Plekhanov <pl...@gmail.com>
---
.../processors/query/calcite/rel/IgniteLimit.java | 15 +-
.../integration/AbstractBasicIntegrationTest.java | 7 +-
.../query/calcite/integration/FunctionsTest.java | 3 -
.../LimitOffsetIntegrationTest.java} | 169 +++++----------------
.../calcite/planner/LimitOffsetPlannerTest.java | 6 +
.../ignite/testsuites/IntegrationTestSuite.java | 4 +-
6 files changed, 57 insertions(+), 147 deletions(-)
diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteLimit.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteLimit.java
index 3bd29161807..ae3f91bcad3 100644
--- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteLimit.java
+++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteLimit.java
@@ -136,24 +136,19 @@ public class IgniteLimit extends SingleRel implements IgniteRel {
/** {@inheritDoc} */
@Override public RelOptCost computeSelfCost(RelOptPlanner planner, RelMetadataQuery mq) {
- double inputRowCount = mq.getRowCount(getInput());
-
- double lim = fetch != null ? doubleFromRex(fetch, inputRowCount * FETCH_IS_PARAM_FACTOR) : inputRowCount;
- double off = offset != null ? doubleFromRex(offset, inputRowCount * OFFSET_IS_PARAM_FACTOR) : 0;
-
- double rows = Math.min(lim + off, inputRowCount);
+ double rows = estimateRowCount(mq);
return planner.getCostFactory().makeCost(rows, rows * IgniteCost.ROW_PASS_THROUGH_COST, 0);
}
/** {@inheritDoc} */
@Override public double estimateRowCount(RelMetadataQuery mq) {
- double inputRowCount = mq.getRowCount(getInput());
+ double inputRowCnt = mq.getRowCount(getInput());
- double lim = fetch != null ? doubleFromRex(fetch, inputRowCount * FETCH_IS_PARAM_FACTOR) : inputRowCount;
- double off = offset != null ? doubleFromRex(offset, inputRowCount * OFFSET_IS_PARAM_FACTOR) : 0;
+ double lim = fetch != null ? doubleFromRex(fetch, inputRowCnt * FETCH_IS_PARAM_FACTOR) : inputRowCnt;
+ double off = offset != null ? doubleFromRex(offset, inputRowCnt * OFFSET_IS_PARAM_FACTOR) : 0;
- return Math.min(lim, inputRowCount - off);
+ return Math.max(0, Math.min(lim, inputRowCnt - off));
}
/**
diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/AbstractBasicIntegrationTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/AbstractBasicIntegrationTest.java
index 8e3a38476b7..94086d33cad 100644
--- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/AbstractBasicIntegrationTest.java
+++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/AbstractBasicIntegrationTest.java
@@ -60,6 +60,9 @@ import static org.apache.ignite.testframework.GridTestUtils.waitForCondition;
*/
@WithSystemProperty(key = "calcite.debug", value = "false")
public class AbstractBasicIntegrationTest extends GridCommonAbstractTest {
+ /** */
+ protected static final Object[] NULL_RESULT = new Object[] { null };
+
/** */
protected static final String TABLE_NAME = "person";
@@ -147,8 +150,8 @@ public class AbstractBasicIntegrationTest extends GridCommonAbstractTest {
* @param cls Exception class.
* @param msg Error message.
*/
- protected void assertThrows(String sql, Class<? extends Exception> cls, String msg) {
- assertThrowsAnyCause(log, () -> executeSql(sql), cls, msg);
+ protected void assertThrows(String sql, Class<? extends Exception> cls, String msg, Object... args) {
+ assertThrowsAnyCause(log, () -> executeSql(sql, args), cls, msg);
}
/** */
diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/FunctionsTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/FunctionsTest.java
index c6939c31d92..2d33342fd2c 100644
--- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/FunctionsTest.java
+++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/FunctionsTest.java
@@ -35,9 +35,6 @@ import org.junit.Test;
* Test Ignite SQL functions.
*/
public class FunctionsTest extends AbstractBasicIntegrationTest {
- /** */
- private static final Object[] NULL_RESULT = new Object[] { null };
-
/** */
@Test
public void testTimestampDiffWithFractionsOfSecond() {
diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/LimitOffsetTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/LimitOffsetIntegrationTest.java
similarity index 59%
rename from modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/LimitOffsetTest.java
rename to modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/LimitOffsetIntegrationTest.java
index 266f2c7773f..aa9ba97e750 100644
--- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/LimitOffsetTest.java
+++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/LimitOffsetIntegrationTest.java
@@ -15,7 +15,7 @@
* limitations under the License.
*/
-package org.apache.ignite.internal.processors.query.calcite;
+package org.apache.ignite.internal.processors.query.calcite.integration;
import java.math.BigDecimal;
import java.util.Arrays;
@@ -24,18 +24,13 @@ import org.apache.calcite.sql.validate.SqlValidatorException;
import org.apache.ignite.IgniteCache;
import org.apache.ignite.cache.CacheMode;
import org.apache.ignite.cache.QueryEntity;
-import org.apache.ignite.cache.query.FieldsQueryCursor;
import org.apache.ignite.configuration.CacheConfiguration;
import org.apache.ignite.configuration.IgniteConfiguration;
-import org.apache.ignite.internal.IgniteEx;
import org.apache.ignite.internal.processors.query.IgniteSQLException;
-import org.apache.ignite.internal.processors.query.QueryEngine;
import org.apache.ignite.internal.processors.query.calcite.exec.rel.AbstractNode;
-import org.apache.ignite.internal.processors.query.calcite.util.Commons;
import org.apache.ignite.internal.util.typedef.F;
import org.apache.ignite.internal.util.typedef.X;
import org.apache.ignite.internal.util.typedef.internal.U;
-import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
import org.junit.Test;
import static java.util.Collections.singletonList;
@@ -43,7 +38,7 @@ import static java.util.Collections.singletonList;
/**
* Limit / offset tests.
*/
-public class LimitOffsetTest extends GridCommonAbstractTest {
+public class LimitOffsetIntegrationTest extends AbstractBasicIntegrationTest {
/** */
private static IgniteCache<Integer, String> cacheRepl;
@@ -51,11 +46,14 @@ public class LimitOffsetTest extends GridCommonAbstractTest {
private static IgniteCache<Integer, String> cachePart;
/** {@inheritDoc} */
- @Override protected void beforeTestsStarted() throws Exception {
- startGrids(2);
+ @Override protected void beforeTest() throws Exception {
+ cacheRepl = client.cache("TEST_REPL");
+ cachePart = client.cache("TEST_PART");
+ }
- cacheRepl = grid(0).cache("TEST_REPL");
- cachePart = grid(0).cache("TEST_PART");
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ // Override method to keep caches after tests.
}
/** {@inheritDoc} */
@@ -76,7 +74,7 @@ public class LimitOffsetTest extends GridCommonAbstractTest {
.setKeyFieldName("id")
.setValueFieldName("val")
.addQueryField("id", Integer.class.getName(), null)
- .addQueryField("val", String.class.getName(), null);;
+ .addQueryField("val", String.class.getName(), null);
return super.getConfiguration(igniteInstanceName)
.setCacheConfiguration(
@@ -93,117 +91,35 @@ public class LimitOffsetTest extends GridCommonAbstractTest {
/** Tests correctness of fetch / offset params. */
@Test
public void testInvalidLimitOffset() {
- QueryEngine engine = engine(grid(0));
-
String bigInt = BigDecimal.valueOf(10000000000L).toString();
- try {
- List<FieldsQueryCursor<List<?>>> cursors =
- engine.query(null, "PUBLIC",
- "SELECT * FROM TEST_REPL OFFSET " + bigInt + " ROWS");
- cursors.get(0).getAll();
-
- fail();
- }
- catch (Throwable e) {
- assertTrue(e.toString(), X.hasCause(e, "Illegal value of offset", SqlValidatorException.class));
- }
-
- try {
- List<FieldsQueryCursor<List<?>>> cursors =
- engine.query(null, "PUBLIC",
- "SELECT * FROM TEST_REPL FETCH FIRST " + bigInt + " ROWS ONLY");
- cursors.get(0).getAll();
-
- fail();
- }
- catch (Throwable e) {
- assertTrue(e.toString(), X.hasCause(e, "Illegal value of fetch / limit", SqlValidatorException.class));
- }
-
- try {
- List<FieldsQueryCursor<List<?>>> cursors =
- engine.query(null, "PUBLIC", "SELECT * FROM TEST_REPL LIMIT " + bigInt);
- cursors.get(0).getAll();
+ assertThrows("SELECT * FROM TEST_REPL OFFSET " + bigInt + " ROWS",
+ SqlValidatorException.class, "Illegal value of offset");
- fail();
- }
- catch (Throwable e) {
- assertTrue(e.toString(), X.hasCause(e, "Illegal value of fetch / limit", SqlValidatorException.class));
- }
+ assertThrows("SELECT * FROM TEST_REPL FETCH FIRST " + bigInt + " ROWS ONLY",
+ SqlValidatorException.class, "Illegal value of fetch / limit");
- try {
- List<FieldsQueryCursor<List<?>>> cursors =
- engine.query(null, "PUBLIC",
- "SELECT * FROM TEST_REPL OFFSET -1 ROWS FETCH FIRST -1 ROWS ONLY");
- cursors.get(0).getAll();
+ assertThrows("SELECT * FROM TEST_REPL LIMIT " + bigInt,
+ SqlValidatorException.class, "Illegal value of fetch / limit");
- fail();
- }
- catch (Throwable e) {
- assertTrue(e.toString(), X.hasCause(e, IgniteSQLException.class));
- }
+ assertThrows("SELECT * FROM TEST_REPL OFFSET -1 ROWS FETCH FIRST -1 ROWS ONLY",
+ IgniteSQLException.class, null);
- try {
- List<FieldsQueryCursor<List<?>>> cursors =
- engine.query(null, "PUBLIC",
- "SELECT * FROM TEST_REPL OFFSET -1 ROWS");
- cursors.get(0).getAll();
+ assertThrows("SELECT * FROM TEST_REPL OFFSET -1 ROWS",
+ IgniteSQLException.class, null);
- fail();
- }
- catch (Throwable e) {
- assertTrue(e.toString(), X.hasCause(e, IgniteSQLException.class));
- }
-
- try {
- List<FieldsQueryCursor<List<?>>> cursors =
- engine.query(null, "PUBLIC",
- "SELECT * FROM TEST_REPL OFFSET 2+1 ROWS");
- cursors.get(0).getAll();
-
- fail();
- }
- catch (Throwable e) {
- assertTrue(e.toString(), X.hasCause(e, IgniteSQLException.class));
- }
+ assertThrows("SELECT * FROM TEST_REPL OFFSET 2+1 ROWS",
+ IgniteSQLException.class, null);
// Check with parameters
- try {
- List<FieldsQueryCursor<List<?>>> cursors =
- engine.query(null, "PUBLIC",
- "SELECT * FROM TEST_REPL OFFSET ? ROWS FETCH FIRST ? ROWS ONLY", -1, -1);
- cursors.get(0).getAll();
+ assertThrows("SELECT * FROM TEST_REPL OFFSET ? ROWS FETCH FIRST ? ROWS ONLY",
+ SqlValidatorException.class, "Illegal value of fetch / limit", -1, -1);
- fail();
- }
- catch (Throwable e) {
- assertTrue(e.toString(), X.hasCause(e, "Illegal value of fetch / limit", SqlValidatorException.class));
- }
-
- try {
- List<FieldsQueryCursor<List<?>>> cursors =
- engine.query(null, "PUBLIC",
- "SELECT * FROM TEST_REPL OFFSET ? ROWS", -1);
- cursors.get(0).getAll();
-
- fail();
- }
- catch (Throwable e) {
- assertTrue(e.toString(), X.hasCause(e, "Illegal value of offset", SqlValidatorException.class));
- }
+ assertThrows("SELECT * FROM TEST_REPL OFFSET ? ROWS",
+ SqlValidatorException.class, "Illegal value of offset", -1);
- try {
- List<FieldsQueryCursor<List<?>>> cursors =
- engine.query(null, "PUBLIC",
- "SELECT * FROM TEST_REPL FETCH FIRST ? ROWS ONLY", -1);
- cursors.get(0).getAll();
-
- fail();
- }
- catch (Throwable e) {
- assertTrue(e.toString(), X.hasCause(e, "Illegal value of fetch / limit", SqlValidatorException.class));
- }
+ assertThrows("SELECT * FROM TEST_REPL FETCH FIRST ? ROWS ONLY",
+ SqlValidatorException.class, "Illegal value of fetch / limit", -1);
}
/**
@@ -239,13 +155,8 @@ public class LimitOffsetTest extends GridCommonAbstractTest {
public void testLimitDistributed() throws Exception {
fillCache(cachePart, 10_000);
- QueryEngine engine = engine(grid(0));
-
for (String order : F.asArray("id", "val")) { // Order by ID - without explicit IgniteSort node.
- FieldsQueryCursor<List<?>> cur = engine.query(null, "PUBLIC",
- "SELECT id FROM TEST_PART ORDER BY " + order + " LIMIT 1000 OFFSET 5000").get(0);
-
- List<List<?>> res = cur.getAll();
+ List<List<?>> res = sql("SELECT id FROM TEST_PART ORDER BY " + order + " LIMIT 1000 OFFSET 5000");
assertEquals(1000, res.size());
@@ -254,11 +165,19 @@ public class LimitOffsetTest extends GridCommonAbstractTest {
}
}
+ /** */
+ @Test
+ public void testOffsetOutOfRange() throws Exception {
+ fillCache(cachePart, 5);
+
+ assertQuery("SELECT (SELECT id FROM TEST_PART ORDER BY id LIMIT 1 OFFSET 10)").returns(NULL_RESULT).check();
+ }
+
/**
* @param c Cache.
* @param rows Rows count.
*/
- private void fillCache(IgniteCache c, int rows) throws InterruptedException {
+ private void fillCache(IgniteCache<Integer, String> c, int rows) throws InterruptedException {
c.clear();
for (int i = 0; i < rows; ++i)
@@ -277,8 +196,6 @@ public class LimitOffsetTest extends GridCommonAbstractTest {
* @param sorted Use sorted query (adds ORDER BY).
*/
void checkQuery(int rows, int lim, int off, boolean param, boolean sorted) {
- QueryEngine engine = engine(grid(0));
-
String sql = createSql(lim, off, param, sorted);
Object[] params;
@@ -293,10 +210,7 @@ public class LimitOffsetTest extends GridCommonAbstractTest {
log.info("SQL: " + sql + (param ? "params=" + Arrays.toString(params) : ""));
- List<FieldsQueryCursor<List<?>>> cursors =
- engine.query(null, "PUBLIC", sql, param ? params : X.EMPTY_OBJECT_ARRAY);
-
- List<List<?>> res = cursors.get(0).getAll();
+ List<List<?>> res = sql(sql, params);
assertEquals("Invalid results size. [rows=" + rows + ", limit=" + lim + ", off=" + off
+ ", res=" + res.size() + ']', expectedSize(rows, lim, off), res.size());
@@ -341,9 +255,4 @@ public class LimitOffsetTest extends GridCommonAbstractTest {
return sb.toString();
}
-
- /** */
- private QueryEngine engine(IgniteEx grid) {
- return Commons.lookupComponent(grid.context(), QueryEngine.class);
- }
}
diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/LimitOffsetPlannerTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/LimitOffsetPlannerTest.java
index 3e0f38aeb30..c773cb4f7ce 100644
--- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/LimitOffsetPlannerTest.java
+++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/LimitOffsetPlannerTest.java
@@ -77,6 +77,12 @@ public class LimitOffsetPlannerTest extends AbstractPlannerTest {
assertPlan("SELECT * FROM TEST ORDER BY ID OFFSET 60", publicSchema,
nodeOrAnyChild(isInstanceOf(IgniteLimit.class)
.and(l -> l.getCluster().getMetadataQuery().getRowCount(l) == ROW_CNT - 60d)));
+
+ assertPlan("SELECT * FROM TEST ORDER BY ID LIMIT 1 OFFSET " + ROW_CNT * 2, publicSchema,
+ nodeOrAnyChild(isInstanceOf(IgniteLimit.class)
+ // Estimated row count returned by IgniteLimit node is 0, but after validation it becomes 1
+ // if it less than 1.
+ .and(l -> l.getCluster().getMetadataQuery().getRowCount(l) == 1)));
}
/** */
diff --git a/modules/calcite/src/test/java/org/apache/ignite/testsuites/IntegrationTestSuite.java b/modules/calcite/src/test/java/org/apache/ignite/testsuites/IntegrationTestSuite.java
index 44a6113e9bf..198d5743646 100644
--- a/modules/calcite/src/test/java/org/apache/ignite/testsuites/IntegrationTestSuite.java
+++ b/modules/calcite/src/test/java/org/apache/ignite/testsuites/IntegrationTestSuite.java
@@ -20,7 +20,6 @@ package org.apache.ignite.testsuites;
import org.apache.ignite.internal.processors.query.calcite.CalciteQueryProcessorTest;
import org.apache.ignite.internal.processors.query.calcite.CancelTest;
import org.apache.ignite.internal.processors.query.calcite.DateTimeTest;
-import org.apache.ignite.internal.processors.query.calcite.LimitOffsetTest;
import org.apache.ignite.internal.processors.query.calcite.SqlFieldsQueryUsageTest;
import org.apache.ignite.internal.processors.query.calcite.UnstableTopologyTest;
import org.apache.ignite.internal.processors.query.calcite.integration.AggregatesIntegrationTest;
@@ -39,6 +38,7 @@ import org.apache.ignite.internal.processors.query.calcite.integration.JoinInteg
import org.apache.ignite.internal.processors.query.calcite.integration.KeepBinaryIntegrationTest;
import org.apache.ignite.internal.processors.query.calcite.integration.KillCommandDdlIntegrationTest;
import org.apache.ignite.internal.processors.query.calcite.integration.KillQueryCommandDdlIntegrationTest;
+import org.apache.ignite.internal.processors.query.calcite.integration.LimitOffsetIntegrationTest;
import org.apache.ignite.internal.processors.query.calcite.integration.LocalDateTimeSupportTest;
import org.apache.ignite.internal.processors.query.calcite.integration.MemoryQuotasIntegrationTest;
import org.apache.ignite.internal.processors.query.calcite.integration.MetadataIntegrationTest;
@@ -78,7 +78,7 @@ import org.junit.runners.Suite;
CalciteBasicSecondaryIndexIntegrationTest.class,
CancelTest.class,
DateTimeTest.class,
- LimitOffsetTest.class,
+ LimitOffsetIntegrationTest.class,
SqlFieldsQueryUsageTest.class,
AggregatesIntegrationTest.class,
MetadataIntegrationTest.class,