From 81d577db522446d3a2b9e9eebce566102dd5679f Mon Sep 17 00:00:00 2001 From: Lantao Jin Date: Tue, 14 Oct 2025 12:29:03 +0800 Subject: [PATCH] [FollowUp] Set 0 and negative value of subsearch.maxout as unlimited (#4534) * [FollowUp] Set 0 and negative value of subsearch.maxout as unlimited Signed-off-by: Lantao Jin * fix doctest Signed-off-by: Lantao Jin * Fix conflicts Signed-off-by: Lantao Jin --------- Signed-off-by: Lantao Jin (cherry picked from commit de2fdc87db56c5e26a86c25713edff746d5e35ac) --- .../sql/calcite/CalciteRelNodeVisitor.java | 4 ++-- .../sql/calcite/CalciteRexNodeVisitor.java | 7 +----- .../org/opensearch/sql/calcite/SysLimit.java | 2 +- docs/user/ppl/admin/settings.rst | 8 +++---- docs/user/ppl/cmd/join.rst | 2 +- docs/user/ppl/cmd/subquery.rst | 4 ++-- .../remote/CalcitePPLExistsSubqueryIT.java | 22 +++++-------------- .../remote/CalcitePPLInSubqueryIT.java | 4 ++-- .../remote/CalcitePPLScalarSubqueryIT.java | 4 ++-- .../standalone/MapConcatFunctionIT.java | 1 - .../setting/OpenSearchSettings.java | 2 -- .../calcite/CalcitePPLExistsSubqueryTest.java | 20 +++++------------ 12 files changed, 26 insertions(+), 54 deletions(-) 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 d14b4c15859..90f5d9551a1 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java @@ -1141,8 +1141,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 afbaf0d665d..19587f6b0b5 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalciteRexNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalciteRexNodeVisitor.java @@ -517,9 +517,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 = @@ -537,10 +536,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 329a7a9c19f..50eb8bf4f95 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/SysLimit.java +++ b/core/src/main/java/org/opensearch/sql/calcite/SysLimit.java @@ -39,7 +39,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 21aaa0e92e9..1060e99c0c4 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 @@ -393,7 +393,7 @@ public void testInCorrelatedSubqueryMaxOut() throws IOException { } @Test - public void testSubsearchMaxOutZero() throws IOException { + public void testSubsearchMaxOutZeroMeansUnlimited() throws IOException { setSubsearchMaxOut(0); JSONObject result = executeQuery( @@ -406,7 +406,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 70143172d27..d3d65b73065 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 @@ -28,7 +28,6 @@ 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; import org.opensearch.sql.expression.function.BuiltinFunctionName; import org.opensearch.sql.expression.function.PPLFuncImpTable; 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 802b462b84d..ef803bea572 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 @@ -149,7 +149,6 @@ public class OpenSearchSettings extends Settings { Setting.intSetting( Key.PPL_SUBSEARCH_MAXOUT.getKeyValue(), 10000, - -1, Setting.Property.NodeScope, Setting.Property.Dynamic); @@ -157,7 +156,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 5bb9ad41ead..806fca5036f 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 @@ -689,7 +689,7 @@ public void testSubsearchMaxOutUncorrelated2() { } @Test - public void testSubsearchMaxOutZero() { + public void testSubsearchMaxOutZeroMeansUnlimited() { doReturn(0).when(settings).getSettingValue(Settings.Key.PPL_SUBSEARCH_MAXOUT); String ppl = "source=EMP\n" @@ -704,7 +704,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); @@ -713,14 +714,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 = "source=EMP\n" @@ -740,14 +741,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); } }