diff --git a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java index 035aea6f4c9..f161b9fc4ab 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java @@ -1138,8 +1138,8 @@ private Optional extractAliasLiteral(RexNode node) { public RelNode visitJoin(Join node, CalcitePlanContext context) { List children = node.getChildren(); children.forEach(c -> analyze(c, context)); - // add join.subsearch_maxout limit to subsearch side - if (context.sysLimit.joinSubsearchLimit() >= 0) { + // add join.subsearch_maxout limit to subsearch side, 0 and negative means unlimited. + if (context.sysLimit.joinSubsearchLimit() > 0) { PlanUtils.replaceTop( context.relBuilder, LogicalSystemLimit.create( diff --git a/core/src/main/java/org/opensearch/sql/calcite/CalciteRexNodeVisitor.java b/core/src/main/java/org/opensearch/sql/calcite/CalciteRexNodeVisitor.java index 8e06894c2b3..bb6f16b2e8a 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalciteRexNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalciteRexNodeVisitor.java @@ -516,9 +516,8 @@ private RelNode resolveSubqueryPlan( context.setResolvingJoinCondition(false); } subquery.accept(planVisitor, context); - + // add subsearch.maxout limit to exists-in subsearch, 0 and negative means unlimited if (context.sysLimit.subsearchLimit() > 0 && !(subqueryExpression instanceof ScalarSubquery)) { - // Add subsearch.maxout limit to exists-in subsearch: // Cannot add system limit to the top of subquery simply. // Instead, add system limit under the correlated conditions. SubsearchUtils.SystemLimitInsertionShuttle shuttle = @@ -536,10 +535,6 @@ private RelNode resolveSubqueryPlan( } // pop the inner plan RelNode subqueryRel = context.relBuilder.build(); - // if maxout = 0, return empty results - if (context.sysLimit.subsearchLimit() == 0) { - subqueryRel = context.relBuilder.values(subqueryRel.getRowType()).build(); - } // clear the exists subquery resolving state // restore to the previous state if (isResolvingJoinConditionOuter) { diff --git a/core/src/main/java/org/opensearch/sql/calcite/SysLimit.java b/core/src/main/java/org/opensearch/sql/calcite/SysLimit.java index 2a17eb31bba..4f2cd951079 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/SysLimit.java +++ b/core/src/main/java/org/opensearch/sql/calcite/SysLimit.java @@ -19,7 +19,7 @@ public static SysLimit fromSettings(Settings settings) { } /** No limitation on subsearch */ - public static SysLimit UNLIMITED_SUBSEARCH = new SysLimit(10000, -1, -1); + public static SysLimit UNLIMITED_SUBSEARCH = new SysLimit(10000, 0, 0); /** For testing only */ public static SysLimit DEFAULT = new SysLimit(10000, 10000, 50000); diff --git a/docs/user/ppl/admin/settings.rst b/docs/user/ppl/admin/settings.rst index 13e8f3d7185..9b4aec17771 100644 --- a/docs/user/ppl/admin/settings.rst +++ b/docs/user/ppl/admin/settings.rst @@ -290,7 +290,7 @@ plugins.ppl.subsearch.maxout Description ----------- -The size configures the maximum of rows to return from subsearch. The default value is: ``10000``. A value of ``-1`` indicates that the restriction is unlimited. +The size configures the maximum of rows to return from subsearch. The default value is: ``10000``. A value of ``0`` indicates that the restriction is unlimited. Version ------- @@ -303,14 +303,14 @@ Change the subsearch.maxout to unlimited:: sh$ curl -sS -H 'Content-Type: application/json' \ ... -X PUT localhost:9200/_plugins/_query/settings \ - ... -d '{"persistent" : {"plugins.ppl.subsearch.maxout" : "-1"}}' + ... -d '{"persistent" : {"plugins.ppl.subsearch.maxout" : "0"}}' { "acknowledged": true, "persistent": { "plugins": { "ppl": { "subsearch": { - "maxout": "-1" + "maxout": "0" } } } @@ -324,7 +324,7 @@ plugins.ppl.join.subsearch_maxout Description ----------- -The size configures the maximum of rows from subsearch to join against. This configuration impacts ``join`` command. The default value is: ``50000``. A value of ``-1`` indicates that the restriction is unlimited. +The size configures the maximum of rows from subsearch to join against. This configuration impacts ``join`` command. The default value is: ``50000``. A value of ``0`` indicates that the restriction is unlimited. Version ------- diff --git a/docs/user/ppl/cmd/join.rst b/docs/user/ppl/cmd/join.rst index fd596e1d568..3b986071261 100644 --- a/docs/user/ppl/cmd/join.rst +++ b/docs/user/ppl/cmd/join.rst @@ -71,7 +71,7 @@ Result set:: plugins.ppl.join.subsearch_maxout --------------------------------- -The size configures the maximum of rows from subsearch to join against. The default value is: ``50000``. A value of ``-1`` indicates that the restriction is unlimited. +The size configures the maximum of rows from subsearch to join against. The default value is: ``50000``. A value of ``0`` indicates that the restriction is unlimited. Change the join.subsearch_maxout to 5000:: diff --git a/docs/user/ppl/cmd/subquery.rst b/docs/user/ppl/cmd/subquery.rst index 98ee6c28157..f7883202c70 100644 --- a/docs/user/ppl/cmd/subquery.rst +++ b/docs/user/ppl/cmd/subquery.rst @@ -73,13 +73,13 @@ Result set:: plugins.ppl.subsearch.maxout ---------------------------- -The size configures the maximum of rows to return from subsearch. The default value is: ``10000``. A value of ``-1`` indicates that the restriction is unlimited. +The size configures the maximum of rows to return from subsearch. The default value is: ``10000``. A value of ``0`` indicates that the restriction is unlimited. Change the subsearch.maxout to unlimited:: sh$ curl -sS -H 'Content-Type: application/json' \ ... -X PUT localhost:9200/_plugins/_query/settings \ - ... -d '{"persistent" : {"plugins.ppl.subsearch.maxout" : "-1"}}' + ... -d '{"persistent" : {"plugins.ppl.subsearch.maxout" : "0"}}' { "acknowledged": true, "persistent": { diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLExistsSubqueryIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLExistsSubqueryIT.java index 5eb930bb730..73e5c49987b 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLExistsSubqueryIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLExistsSubqueryIT.java @@ -378,7 +378,7 @@ public void testSubsearchMaxOutUncorrelated() throws IOException { } @Test - public void testSubsearchMaxOutZero1() throws IOException { + public void testUncorrelatedSubsearchMaxOutZeroMeansUnlimited() throws IOException { setSubsearchMaxOut(0); JSONObject result = executeQuery( @@ -390,24 +390,12 @@ public void testSubsearchMaxOutZero1() throws IOException { + "| sort - salary" + "| fields id, name, salary", TEST_INDEX_WORKER, TEST_INDEX_WORK_INFORMATION)); - verifyNumOfRows(result, 0); - - result = - executeQuery( - String.format( - "source = %s" - + "| where not exists [" - + " source = %s | where name = 'Tom'" - + " ]" - + "| sort - salary" - + "| fields id, name, salary", - TEST_INDEX_WORKER, TEST_INDEX_WORK_INFORMATION)); verifyNumOfRows(result, 7); resetSubsearchMaxOut(); } @Test - public void testSubsearchMaxOutZero2() throws IOException { + public void testCorrelatedSubsearchMaxOutZeroMeansUnlimited() throws IOException { setSubsearchMaxOut(0); JSONObject result = executeQuery( @@ -419,7 +407,7 @@ public void testSubsearchMaxOutZero2() throws IOException { + "| sort - salary" + "| fields id, name, salary", TEST_INDEX_WORKER, TEST_INDEX_WORK_INFORMATION)); - verifyNumOfRows(result, 0); + verifyNumOfRows(result, 5); result = executeQuery( String.format( @@ -430,12 +418,12 @@ public void testSubsearchMaxOutZero2() throws IOException { + "| sort - salary" + "| fields id, name, salary", TEST_INDEX_WORKER, TEST_INDEX_WORK_INFORMATION)); - verifyNumOfRows(result, 7); + verifyNumOfRows(result, 2); resetSubsearchMaxOut(); } @Test - public void testSubsearchMaxOutUnlimited() throws IOException { + public void testSubsearchMaxOutNegativeMeansUnlimited() throws IOException { setSubsearchMaxOut(-1); JSONObject result = executeQuery( diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLInSubqueryIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLInSubqueryIT.java index 119a251558f..d417421b5a4 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLInSubqueryIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLInSubqueryIT.java @@ -403,7 +403,7 @@ public void testInCorrelatedSubqueryMaxOut() throws IOException { } @Test - public void testSubsearchMaxOutZero() throws IOException { + public void testSubsearchMaxOutZeroMeansUnlimited() throws IOException { setSubsearchMaxOut(0); JSONObject result = executeQuery( @@ -416,7 +416,7 @@ public void testSubsearchMaxOutZero() throws IOException { + "| fields id, name, salary", TEST_INDEX_WORKER, TEST_INDEX_WORK_INFORMATION)); verifySchema(result, schema("id", "int"), schema("name", "string"), schema("salary", "int")); - verifyNumOfRows(result, 0); + verifyNumOfRows(result, 5); resetSubsearchMaxOut(); } } diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLScalarSubqueryIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLScalarSubqueryIT.java index 9fc4f8b2b12..133530bbd7b 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLScalarSubqueryIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLScalarSubqueryIT.java @@ -311,7 +311,7 @@ public void testNestedScalarSubqueryWithTableAlias() throws IOException { } @Test - public void testSubsearchMaxOutZero() throws IOException { + public void testSubsearchMaxOutZeroMeansUnlimited() throws IOException { setSubsearchMaxOut(0); JSONObject result = executeQuery( @@ -322,7 +322,7 @@ public void testSubsearchMaxOutZero() throws IOException { + " ]" + "| fields id, name", TEST_INDEX_WORKER, TEST_INDEX_WORK_INFORMATION)); - verifyNumOfRows(result, 0); + verifyNumOfRows(result, 5); resetSubsearchMaxOut(); } } diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/standalone/MapConcatFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/standalone/MapConcatFunctionIT.java index 29c27da354f..81bec92f0c9 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/standalone/MapConcatFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/standalone/MapConcatFunctionIT.java @@ -26,6 +26,7 @@ import org.apache.calcite.tools.RelBuilder; import org.junit.jupiter.api.Test; import org.opensearch.sql.calcite.CalcitePlanContext; +import org.opensearch.sql.calcite.SysLimit; import org.opensearch.sql.calcite.utils.CalciteToolsHelper.OpenSearchRelRunners; import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.executor.QueryType; @@ -175,6 +176,6 @@ private CalcitePlanContext createCalcitePlanContext() { Settings settings = getSettings(); return CalcitePlanContext.create( - config.build(), settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT), QueryType.PPL); + config.build(), SysLimit.fromSettings(settings), QueryType.PPL); } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java index c201408255a..db397b836d0 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java @@ -133,7 +133,6 @@ public class OpenSearchSettings extends Settings { Setting.intSetting( Key.PPL_SUBSEARCH_MAXOUT.getKeyValue(), 10000, - -1, Setting.Property.NodeScope, Setting.Property.Dynamic); @@ -141,7 +140,6 @@ public class OpenSearchSettings extends Settings { Setting.intSetting( Key.PPL_JOIN_SUBSEARCH_MAXOUT.getKeyValue(), 50000, - -1, Setting.Property.NodeScope, Setting.Property.Dynamic); diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLExistsSubqueryTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLExistsSubqueryTest.java index 99e59289e9c..76c280db92f 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLExistsSubqueryTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLExistsSubqueryTest.java @@ -725,7 +725,7 @@ public void testSubsearchMaxOutUncorrelated2() { } @Test - public void testSubsearchMaxOutZero() { + public void testSubsearchMaxOutZeroMeansUnlimited() { doReturn(0).when(settings).getSettingValue(Settings.Key.PPL_SUBSEARCH_MAXOUT); String ppl = """ @@ -742,7 +742,8 @@ public void testSubsearchMaxOutZero() { + "LogicalProject(EMPNO=[$0], ENAME=[$1])\n" + " LogicalSort(sort0=[$0], dir0=[DESC-nulls-last])\n" + " LogicalFilter(condition=[EXISTS({\n" - + "LogicalValues(tuples=[[]])\n" + + "LogicalFilter(condition=[AND(=($cor0.SAL, $2), >($1, 1000.0:DECIMAL(5, 1)))])\n" + + " LogicalTableScan(table=[[scott, SALGRADE]])\n" + "})], variablesSet=[[$cor0]])\n" + " LogicalTableScan(table=[[scott, EMP]])\n"; verifyLogical(root, expectedLogical); @@ -751,14 +752,14 @@ public void testSubsearchMaxOutZero() { "SELECT `EMPNO`, `ENAME`\n" + "FROM `scott`.`EMP`\n" + "WHERE EXISTS (SELECT *\n" - + "FROM (VALUES (NULL, NULL, NULL)) `t` (`GRADE`, `LOSAL`, `HISAL`)\n" - + "WHERE 1 = 0)\n" + + "FROM `scott`.`SALGRADE`\n" + + "WHERE `EMP`.`SAL` = `HISAL` AND `LOSAL` > 1000.0)\n" + "ORDER BY `EMPNO` DESC"; verifyPPLToSparkSQL(root, expectedSparkSql); } @Test - public void testSubsearchMaxOutUnlimited() { + public void testSubsearchMaxOutNegativeMeansUnlimited() { doReturn(-1).when(settings).getSettingValue(Settings.Key.PPL_SUBSEARCH_MAXOUT); String ppl = """ @@ -780,14 +781,5 @@ public void testSubsearchMaxOutUnlimited() { + "})], variablesSet=[[$cor0]])\n" + " LogicalTableScan(table=[[scott, EMP]])\n"; verifyLogical(root, expectedLogical); - - String expectedSparkSql = - "SELECT `EMPNO`, `ENAME`\n" - + "FROM `scott`.`EMP`\n" - + "WHERE EXISTS (SELECT *\n" - + "FROM `scott`.`SALGRADE`\n" - + "WHERE `EMP`.`SAL` = `HISAL` AND `LOSAL` > 1000.0)\n" - + "ORDER BY `EMPNO` DESC"; - verifyPPLToSparkSQL(root, expectedSparkSql); } }