[Enhancement] Forward QUERY_SIZE_LIMIT to the Analytics Engine unified query path

[RyanL1997]

Description

plugins.query.size_limit (and, structurally, other planning settings) was silently ignored on the Analytics Engine (unified query) path — the AE route always used the hardcoded default (10000) regardless of the configured cluster value. The default (non-AE) pipeline honored the setting correctly.

Root cause. RestUnifiedQueryAction.applyClusterOverrides() forwarded only a hand-picked subset of cluster settings into the UnifiedQueryContext (PPL_REX_MAX_MATCH_LIMIT, PPL_SYNTAX_LEGACY_PREFERRED, MAX_EXPRESSION_DEPTH) and omitted QUERY_SIZE_LIMIT. UnifiedQueryContext.Builder seeds a default settings map (QUERY_SIZE_LIMIT = SysLimit.DEFAULT.querySizeLimit()), so with no forward the hardcoded default was never overwritten. addQuerySizeLimit() then injected a LogicalSystemLimit built from that stale value.

This is a two-lists-drift defect: the builder's default-settings map and the forward list are maintained independently, so any planning setting present in the map but absent from the forward list silently regresses to its default.

Fix

• Replace the hand-maintained forwardClusterSetting calls with a single FORWARDED_CLUSTER_SETTINGS allow-list — one source of truth for which cluster settings the unified path honors — and add QUERY_SIZE_LIMIT to it.
• CALCITE_ENGINE_ENABLED is deliberately excluded: the unified path is Calcite-based by definition and must force it on regardless of the cluster value.

Testing

• Unit tests (RestUnifiedQueryActionTest):
  • clusterQuerySizeLimitReachesAnalyticsContext — the live cluster plugins.query.size_limit reaches the AE plan context's sysLimit. Fails without the fix (expected:<500> but was:<10000>), passes with it.
  • calciteEngineEnabledNotOverriddenByCluster — the Calcite-enabled invariant is not overridden by the cluster value.
• Existing IT is the AE-route regression guard. SettingsIT.testQuerySizeLimit_NoPushdown is not in the parquet exclude list, so it runs on the AE route under -Dtests.analytics.parquet_indices=true. Against a real composite/AE domain it reproduced the bug (expected:<2> but was:<3>), and this fix turns it green — no new IT needed.

Check List

• New functionality includes testing.
• New functionality has been documented in code.
• Commits are signed per the DCO using --signoff.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Ensure consistent Integer type return The mock returns a primitive int (500) but getSettingValue likely returns an Integer object. Ensure type consistency to avoid potential ClassCastException or autoboxing issues. Explicitly return Integer.valueOf(500) for clarity. plugin/src/test/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryActionTest.java [242-244] when(pluginSettings.getSettingValue( org.opensearch.sql.common.setting.Settings.Key.QUERY_SIZE_LIMIT)) - .thenReturn(500); + .thenReturn(Integer.valueOf(500)); Suggestion importance[1-10]: 3 __ Why: While using Integer.valueOf(500) is more explicit, Java's autoboxing handles the conversion automatically. The suggestion is technically correct but offers minimal practical benefit since autoboxing is standard and reliable in modern Java.	Low
	Use Boolean object for consistency The mock returns a primitive boolean (false) but getSettingValue likely returns a Boolean object. Use Boolean.FALSE instead of false to ensure type consistency and avoid potential autoboxing issues. plugin/src/test/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryActionTest.java [259-261] when(pluginSettings.getSettingValue( org.opensearch.sql.common.setting.Settings.Key.CALCITE_ENGINE_ENABLED)) - .thenReturn(false); + .thenReturn(Boolean.FALSE); Suggestion importance[1-10]: 3 __ Why: Similar to the previous suggestion, using Boolean.FALSE instead of false is more explicit but autoboxing handles this automatically. The change provides marginal improvement in code clarity without addressing any actual bug.	Low