Skip to content

Commit de2fdc8

Browse files
LantaoJinLantaoJin
authored
[FollowUp] Set 0 and negative value of subsearch.maxout as unlimited (opensearch-project#4534)
* [FollowUp] Set 0 and negative value of subsearch.maxout as unlimited Signed-off-by: Lantao Jin <ltjin@amazon.com> * fix doctest Signed-off-by: Lantao Jin <ltjin@amazon.com> * Fix conflicts Signed-off-by: Lantao Jin <ltjin@amazon.com> --------- Signed-off-by: Lantao Jin <ltjin@amazon.com>
1 parent 977b7ab commit de2fdc8

12 files changed

Lines changed: 28 additions & 54 deletions

File tree

‎core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1138,8 +1138,8 @@ private Optional<RexLiteral> extractAliasLiteral(RexNode node) {
11381138
public RelNode visitJoin(Join node, CalcitePlanContext context) {
11391139
List<UnresolvedPlan> children = node.getChildren();
11401140
children.forEach(c -> analyze(c, context));
1141-
// add join.subsearch_maxout limit to subsearch side
1142-
if (context.sysLimit.joinSubsearchLimit() >= 0) {
1141+
// add join.subsearch_maxout limit to subsearch side, 0 and negative means unlimited.
1142+
if (context.sysLimit.joinSubsearchLimit() > 0) {
11431143
PlanUtils.replaceTop(
11441144
context.relBuilder,
11451145
LogicalSystemLimit.create(

‎core/src/main/java/org/opensearch/sql/calcite/CalciteRexNodeVisitor.java‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -516,9 +516,8 @@ private RelNode resolveSubqueryPlan(
516516
context.setResolvingJoinCondition(false);
517517
}
518518
subquery.accept(planVisitor, context);
519-
519+
// add subsearch.maxout limit to exists-in subsearch, 0 and negative means unlimited
520520
if (context.sysLimit.subsearchLimit() > 0 && !(subqueryExpression instanceof ScalarSubquery)) {
521-
// Add subsearch.maxout limit to exists-in subsearch:
522521
// Cannot add system limit to the top of subquery simply.
523522
// Instead, add system limit under the correlated conditions.
524523
SubsearchUtils.SystemLimitInsertionShuttle shuttle =
@@ -536,10 +535,6 @@ private RelNode resolveSubqueryPlan(
536535
}
537536
// pop the inner plan
538537
RelNode subqueryRel = context.relBuilder.build();
539-
// if maxout = 0, return empty results
540-
if (context.sysLimit.subsearchLimit() == 0) {
541-
subqueryRel = context.relBuilder.values(subqueryRel.getRowType()).build();
542-
}
543538
// clear the exists subquery resolving state
544539
// restore to the previous state
545540
if (isResolvingJoinConditionOuter) {

‎core/src/main/java/org/opensearch/sql/calcite/SysLimit.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ public static SysLimit fromSettings(Settings settings) {
1919
}
2020

2121
/** No limitation on subsearch */
22-
public static SysLimit UNLIMITED_SUBSEARCH = new SysLimit(10000, -1, -1);
22+
public static SysLimit UNLIMITED_SUBSEARCH = new SysLimit(10000, 0, 0);
2323

2424
/** For testing only */
2525
public static SysLimit DEFAULT = new SysLimit(10000, 10000, 50000);

‎docs/user/ppl/admin/settings.rst‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ plugins.ppl.subsearch.maxout
290290
Description
291291
-----------
292292

293-
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.
293+
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.
294294

295295
Version
296296
-------
@@ -303,14 +303,14 @@ Change the subsearch.maxout to unlimited::
303303

304304
sh$ curl -sS -H 'Content-Type: application/json' \
305305
... -X PUT localhost:9200/_plugins/_query/settings \
306-
... -d '{"persistent" : {"plugins.ppl.subsearch.maxout" : "-1"}}'
306+
... -d '{"persistent" : {"plugins.ppl.subsearch.maxout" : "0"}}'
307307
{
308308
"acknowledged": true,
309309
"persistent": {
310310
"plugins": {
311311
"ppl": {
312312
"subsearch": {
313-
"maxout": "-1"
313+
"maxout": "0"
314314
}
315315
}
316316
}
@@ -324,7 +324,7 @@ plugins.ppl.join.subsearch_maxout
324324
Description
325325
-----------
326326

327-
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.
327+
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.
328328

329329
Version
330330
-------

‎docs/user/ppl/cmd/join.rst‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ Result set::
7171
plugins.ppl.join.subsearch_maxout
7272
---------------------------------
7373

74-
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.
74+
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.
7575

7676
Change the join.subsearch_maxout to 5000::
7777

‎docs/user/ppl/cmd/subquery.rst‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,13 @@ Result set::
7373
plugins.ppl.subsearch.maxout
7474
----------------------------
7575

76-
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.
76+
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.
7777

7878
Change the subsearch.maxout to unlimited::
7979

8080
sh$ curl -sS -H 'Content-Type: application/json' \
8181
... -X PUT localhost:9200/_plugins/_query/settings \
82-
... -d '{"persistent" : {"plugins.ppl.subsearch.maxout" : "-1"}}'
82+
... -d '{"persistent" : {"plugins.ppl.subsearch.maxout" : "0"}}'
8383
{
8484
"acknowledged": true,
8585
"persistent": {

‎integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLExistsSubqueryIT.java‎

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ public void testSubsearchMaxOutUncorrelated() throws IOException {
378378
}
379379

380380
@Test
381-
public void testSubsearchMaxOutZero1() throws IOException {
381+
public void testUncorrelatedSubsearchMaxOutZeroMeansUnlimited() throws IOException {
382382
setSubsearchMaxOut(0);
383383
JSONObject result =
384384
executeQuery(
@@ -390,24 +390,12 @@ public void testSubsearchMaxOutZero1() throws IOException {
390390
+ "| sort - salary"
391391
+ "| fields id, name, salary",
392392
TEST_INDEX_WORKER, TEST_INDEX_WORK_INFORMATION));
393-
verifyNumOfRows(result, 0);
394-
395-
result =
396-
executeQuery(
397-
String.format(
398-
"source = %s"
399-
+ "| where not exists ["
400-
+ " source = %s | where name = 'Tom'"
401-
+ " ]"
402-
+ "| sort - salary"
403-
+ "| fields id, name, salary",
404-
TEST_INDEX_WORKER, TEST_INDEX_WORK_INFORMATION));
405393
verifyNumOfRows(result, 7);
406394
resetSubsearchMaxOut();
407395
}
408396

409397
@Test
410-
public void testSubsearchMaxOutZero2() throws IOException {
398+
public void testCorrelatedSubsearchMaxOutZeroMeansUnlimited() throws IOException {
411399
setSubsearchMaxOut(0);
412400
JSONObject result =
413401
executeQuery(
@@ -419,7 +407,7 @@ public void testSubsearchMaxOutZero2() throws IOException {
419407
+ "| sort - salary"
420408
+ "| fields id, name, salary",
421409
TEST_INDEX_WORKER, TEST_INDEX_WORK_INFORMATION));
422-
verifyNumOfRows(result, 0);
410+
verifyNumOfRows(result, 5);
423411
result =
424412
executeQuery(
425413
String.format(
@@ -430,12 +418,12 @@ public void testSubsearchMaxOutZero2() throws IOException {
430418
+ "| sort - salary"
431419
+ "| fields id, name, salary",
432420
TEST_INDEX_WORKER, TEST_INDEX_WORK_INFORMATION));
433-
verifyNumOfRows(result, 7);
421+
verifyNumOfRows(result, 2);
434422
resetSubsearchMaxOut();
435423
}
436424

437425
@Test
438-
public void testSubsearchMaxOutUnlimited() throws IOException {
426+
public void testSubsearchMaxOutNegativeMeansUnlimited() throws IOException {
439427
setSubsearchMaxOut(-1);
440428
JSONObject result =
441429
executeQuery(

‎integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLInSubqueryIT.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ public void testInCorrelatedSubqueryMaxOut() throws IOException {
403403
}
404404

405405
@Test
406-
public void testSubsearchMaxOutZero() throws IOException {
406+
public void testSubsearchMaxOutZeroMeansUnlimited() throws IOException {
407407
setSubsearchMaxOut(0);
408408
JSONObject result =
409409
executeQuery(
@@ -416,7 +416,7 @@ public void testSubsearchMaxOutZero() throws IOException {
416416
+ "| fields id, name, salary",
417417
TEST_INDEX_WORKER, TEST_INDEX_WORK_INFORMATION));
418418
verifySchema(result, schema("id", "int"), schema("name", "string"), schema("salary", "int"));
419-
verifyNumOfRows(result, 0);
419+
verifyNumOfRows(result, 5);
420420
resetSubsearchMaxOut();
421421
}
422422
}

‎integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLScalarSubqueryIT.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ public void testNestedScalarSubqueryWithTableAlias() throws IOException {
311311
}
312312

313313
@Test
314-
public void testSubsearchMaxOutZero() throws IOException {
314+
public void testSubsearchMaxOutZeroMeansUnlimited() throws IOException {
315315
setSubsearchMaxOut(0);
316316
JSONObject result =
317317
executeQuery(
@@ -322,7 +322,7 @@ public void testSubsearchMaxOutZero() throws IOException {
322322
+ " ]"
323323
+ "| fields id, name",
324324
TEST_INDEX_WORKER, TEST_INDEX_WORK_INFORMATION));
325-
verifyNumOfRows(result, 0);
325+
verifyNumOfRows(result, 5);
326326
resetSubsearchMaxOut();
327327
}
328328
}

‎integ-test/src/test/java/org/opensearch/sql/calcite/standalone/MapConcatFunctionIT.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.apache.calcite.tools.RelBuilder;
2727
import org.junit.jupiter.api.Test;
2828
import org.opensearch.sql.calcite.CalcitePlanContext;
29+
import org.opensearch.sql.calcite.SysLimit;
2930
import org.opensearch.sql.calcite.utils.CalciteToolsHelper.OpenSearchRelRunners;
3031
import org.opensearch.sql.common.setting.Settings;
3132
import org.opensearch.sql.executor.QueryType;
@@ -175,6 +176,6 @@ private CalcitePlanContext createCalcitePlanContext() {
175176

176177
Settings settings = getSettings();
177178
return CalcitePlanContext.create(
178-
config.build(), settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT), QueryType.PPL);
179+
config.build(), SysLimit.fromSettings(settings), QueryType.PPL);
179180
}
180181
}

0 commit comments

Comments
 (0)