feat(ci): add PPL lint rule validation check (eventstats PoC) - #1
Open
Hanyu-W wants to merge 41 commits into
Open
feat(ci): add PPL lint rule validation check (eventstats PoC)#1Hanyu-W wants to merge 41 commits into
Hanyu-W wants to merge 41 commits into
Conversation
…nsearch-project#5563) Signed-off-by: Marc Handalian <marc.handalian@gmail.com>
…t/like/appendpipe) (opensearch-project#5561) Analytics-engine route parity for several PPL IT classes; test-only. Uses the @RequiresCapability annotation + Capability registry (opensearch-project#5560) plus matching excludeTestsMatching entries. CalcitePPLCaseFunctionIT: - Guard the weblogs raw-PUT seeding (appendDataForBadResponse) on a pre-load isIndexExist check — the append-only AE store inflated counts per method. - Skip the otel_logs load on the AE route (multi-value keyword the parquet store rejects); only testNestedCaseAggWithAutoDateHistogram uses it, and that test requires BIN_TIME_FIELD_BUCKETING (bucket column typed string). CalcitePPLStringBuiltinFunctionIT: 7 tests re-PUT a shared _id with different data; the append-only AE store can't replace docs (DELETE unsupported) -> DOC_MUTATION. MultiMatchIT / QueryStringIT / SimpleQueryStringIT wildcard tests: full-text relevance functions with no DataFusion equivalent -> new FULLTEXT_RELEVANCE_FUNC. CalciteLikeQueryIT.test_the_default_3rd_option: AE LIKE is case-insensitive but v2/Calcite is case-sensitive -> new LIKE_CASE_SENSITIVITY. CalcitePPLAppendPipeCommandIT.testDoubleAppendPipeWithFilter: appendpipe drops the main pipeline's rows on the AE route -> new APPENDPIPE_MAIN_RESULT_DROPPED. v2/Calcite route unchanged (all run, 0 skips). Signed-off-by: Kai Huang <ahkcs@amazon.com>
…ype/basic) (opensearch-project#5562) Analytics-engine route parity for four PPL IT classes; test-only. Uses the @RequiresCapability annotation + Capability registry (opensearch-project#5560) plus matching excludeTestsMatching entries. CalciteArrayFunctionIT: - Skip the array-index load on the AE route (multi-value 'numbers' field the parquet store rejects); no test queries it (all build arrays inline). - 16 higher-order lambda functions (transform/mvmap, reduce, filter, exists, forall) -> new ARRAY_HIGHER_ORDER_FUNC (no DataFusion lambda execution). CalcitePPLMapPathIT: - mvcombine lowers to ARRAY_AGG, unregistered on the analytics backend -> new MVCOMBINE_ARRAY_AGG. - addtotals crashes the DataFusion backend with a join panic -> new ADDTOTALS_JOIN_PANIC. CalciteDataTypeIT (guards on base DataTypeIT; build.gradle globs broadened to '*' so they cover the Calcite subclass): - test_nonnumeric_data_types / test_alias_data_type: nested/object/geo/alias types stripped (NESTED_FIELDS). - test_numeric_data_types: scaled_float reported as bigint not double -> new SCALED_FLOAT_TYPE. - testNumericFieldFromString: empty-string -> numeric coerces to null not 0 -> new STRING_TO_NUMERIC_COERCION. - testBooleanFieldFromNumberAcrossWildcardIndices: cross-index incompatible field types rejected -> new CROSS_INDEX_INCOMPATIBLE_TYPES. - testBooleanFieldFromString: seeds+deletes a doc; DELETE unsupported (DOC_MUTATION). CalcitePPLBasicIT.testRegexpFilter: REGEXP filter throws a backend NullPointerException on the AE route -> new REGEXP_FILTER. v2/Calcite route unchanged (all run, 0 skips). Signed-off-by: Kai Huang <ahkcs@amazon.com>
…etime/json/dedup/union/rename/chart) (opensearch-project#5564) Brings 14 PPL IT classes to parity on the analytics-engine route (-Dtests.analytics.parquet_indices=true). Each gated test is skipped ONLY on the AE route via @RequiresCapability + a matching build.gradle excludeTestsMatching; the v2/Calcite path runs all of them unchanged. Engine divergences gated (route-only, behavior is correct elsewhere): - PERCENTILE_APPROXIMATE: percentile/median is approximate on DataFusion - FLOAT_ARITHMETIC_PRECISION: float/half_float arithmetic keeps 32-bit - DATETIME_FORMAT_RENDERING: date_format/strftime token rendering differs - UNIX_TIMESTAMP_SUBSECOND: unix_timestamp drops sub-second precision - JSON_DOLLAR_PATH: json_set/json_delete with $-path is a no-op - DEDUP_NONDETERMINISTIC: dedup surviving-row selection is unstable - SAME_INDEX_UNION_CONFLATION: same-index union conflates (delegate leak) - WILDCARD_COLUMN_ORDER: rename * column order differs - BIN_TIME_FIELD_BUCKETING: span() time bucketing differs - MULTI_VALUE_FIELD_LOAD: otel_logs/game_of_thrones multi-value field can't bulk-load into the parquet store Two cases were harness-predicate gaps, not real divergences, so they now PASS on AE instead of being gated: testStatsPercentileWithMin and testTimestampDiff branch on isCalciteEnabled() for the result type, but the AE route runs the Calcite path while the cluster setting reads false; extended the predicate with isAnalyticsParquetIndicesEnabled(). Guarded the otel_logs / game_of_thrones loads in CalciteChartCommandIT and CalcitePPLJsonBuiltinFunctionIT init() so a multi-value bulk-load failure no longer aborts init() and mislabels unrelated tests. CalciteAnalyticsDatetimeWireFormatIT (AE-only via assumeTrue) updated to assert the date/time UDT types AE now preserves. Results (this batch, on the AE route): - CalcitePPLAggregationIT: 100 run, 2 skip - CalcitePPLBuiltinFunctionIT: 26 run, 3 skip - CalciteDateTimeFunctionIT: 65 run, 5 skip - CalcitePPLDedupIT: 15 run, 3 skip - CalcitePPLJsonBuiltinFunctionIT: 22 run, 2 skip - CalcitePPLRenameIT: 24 run, 1 skip - CalciteUnionCommandIT: 15 run, 2 skip - CalciteChartCommandIT: 15 run, 5 skip - DateTimeFunctionIT: 59 run, 4 skip - StatsCommandIT: 59 run, 6 skip - SystemFunctionIT: 1 run - CalciteAnalyticsDatetimeWireFormatIT: 11 run AE route: 0 failures (was 30+ fail across the batch). V2 baseline: 402 run, 0 fail, 3 pre-existing skips, 0 from these gates. Signed-off-by: Kai Huang <ahkcs@amazon.com>
…roject#5567) * fix: Honor PPL fetch_size on the analytics-engine route PPL's fetch_size caps a response to N rows (no cursor) — the V2 path lowers it to a top-level `head N` in AstStatementBuilder.visitPplStatement. The analytics-engine route bypasses that builder: TransportPPLQueryAction forwards only the query string to RestUnifiedQueryAction.execute(), so fetch_size was dropped and the engine returned the full result set. Thread the request's fetchSize through execute() and apply an equivalent top-level limit on the planned RelNode (addFetchSizeLimit), using the same relBuilder.limit primitive that `head` lowers to. fetchSize <= 0 keeps the prior "system default" behavior; the SQL path (separate cursor-based fetch_size) is unchanged. Before/after (CalcitePPLFetchSizeIT-equivalent, analytics-engine route): before: 9/19 pass (10 fail — fetch_size ignored, full set returned) after: 19/19 pass Signed-off-by: Kai Huang <ahkcs@amazon.com> * chore: spotlessApply on RestUnifiedQueryAction (javadoc reflow + signature wrap) Signed-off-by: Kai Huang <ahkcs@amazon.com> --------- Signed-off-by: Kai Huang <ahkcs@amazon.com>
…ing head (opensearch-project#5518) Signed-off-by: Du Tran <quangdutran809@gmail.com>
…xx) on the SQL path (opensearch-project#5569) Signed-off-by: Jialiang Liang <jiallian@amazon.com>
…ject#5571) Signed-off-by: Chen Dai <daichen@amazon.com>
…s/IP-UDT/metadata/strip-verifier) (opensearch-project#5566) Brings 16 more PPL IT classes to parity on the analytics-engine route (-Dtests.analytics.parquet_indices=true). Route-only divergences are gated AE-only via @RequiresCapability + a matching build.gradle excludeTestsMatching; the v2/Calcite path runs every test unchanged. Strip-verifier (the opensearch-project#5541 guardrail): - AnalyticsUnsupportedFieldStripVerifyIT was failing because 8 datasets carry a multi-value JSON array for a scalar-mapped field, which the parquet store rejects at bulk load. That's a cardinality limitation, not an unsupported field *type*, so it's out of scope for the type strip — the same situation as the existing `join` out-of-scope skip. Added a curated MULTI_VALUE_DATASETS allowlist + safeToSkipForMultiValueLoad that skips only the exact multi-value signature on a known dataset; any other failure still surfaces loudly, and Legs 2-3 still type-check every index that loads. Init-load contamination (not divergences — fixed, not gated): - CalciteWhereCommandIT failed on testDoubleEqual* because init() loaded game_of_thrones (base) and deep_nested (subclass), both multi-value datasets whose bulk-load failure aborted init() and mislabeled the first test. Guarded both loads with isAnalyticsParquetIndicesEnabled(); no test in the hierarchy queries them on the AE route. 32/32 now pass. Non-deterministic sort ties stabilized (not gated — full coverage kept): - The three CalcitePPLSortIT tie tests (testSortWithNullValue, testSortAgeAndFieldsNameAge, testSortWithAutoCast) sorted on a non-unique key, so the tied rows had no defined order and the AE route ordered them differently than the captured Lucene doc order. Added a unique secondary sort key (firstname) to each, making the order deterministic and engine-independent. Verified identical on BOTH routes: 18/18 on AE and 18/18 on v2/Calcite. No gate needed. Engine divergences gated (new capabilities): - INVALID_DATETIME_ERROR_SHAPE: dayname/monthname over an invalid literal throw a different message shape (2 tests) - RAND_SEED_UNSUPPORTED: seeded RAND(seed) is rejected on AE - IP_UDT_BINARY_REPRESENTATION: the IP UDT is materialized as BINARY, so cast(... as IP) and cidrmatch over an IP column fail (2 tests) - TIME_TYPE_WIDENED_TO_TIMESTAMP: a TIME field reads back as TIMESTAMP, defeating TIMEDIFF's [TIME,TIME] signature - BINARY_FIELD_STRIPPED: binary fields are stripped at load - VALUES_LIMIT_NOT_HONORED: values()/list() ignore the configured limit - INDEX_METADATA: _index metadata not exposed (sibling of ID_METADATA) - CROSS_INDEX_OBJECT_LEAF_MERGE: an object leaf in only some wildcard member indices resolves to FIELD_NOT_FOUND - TEXT_KEYWORD_PUSHDOWN_REWRITE: like() doesn't rewrite to .keyword in the explain plan (no Lucene term-pushdown) - LUCENE_PUSHDOWN_EXPLAIN: a test asserting a Lucene SORT-> pushdown fragment can't match the DataFusion plan Reused existing capabilities: - WILDCARD_COLUMN_ORDER: streamstats carries all source columns through; AE returns them in a different column order (values and row order are correct, so a sort can't fix it) (4 CalciteReverseCommandIT tests) - HEAD_WITHOUT_STABLE_SORT: the non-determinism is which rows head N keeps, before the trailing sort, so a sort can't recover it (testHeadThenSort, testAppendWithMergedColumn) - DEDUP_NONDETERMINISTIC: consecutive dedup has no working V2 fallback on the AE route Out of scope: - FieldsCommandIT.testEnhancedFieldsWhenCalciteDisabled asserts the Calcite-DISABLED error; the AE route is always Calcite-enabled. build.gradle exclude only. Results (this batch, on the AE route): 16 classes, 0 failures (was 24 failures), with the 3 sort tests now passing rather than skipped. V2 baseline: 0 failures, only pre-existing/by-design skips (none from these gates). Signed-off-by: Kai Huang <ahkcs@amazon.com>
…ne route with a 4xx (opensearch-project#5570) Signed-off-by: Jialiang Liang <jiallian@amazon.com>
…ct#5565) Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
…for Analytic Engine (opensearch-project#5574) Signed-off-by: Jialiang Liang <jiallian@amazon.com>
…pensearch-project#5575) Signed-off-by: Jialiang Liang <jiallian@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
…pensearch-project#5582) Gate the streamstats ITs that diverge on the analytics-engine route (parquet-backed composite store, DataFusion backend) so the route runs green, while keeping every test active on the v2/Calcite path. Mechanism follows the established capability-gating pattern (opensearch-project#5560): an in-test @RequiresCapability(...) annotation plus a matching integTestRemote excludeTestsMatching entry; both are no-ops on the v2 route. Triaged single-shard and multi-shard (num_shards=3) analytics runs against the v2 baseline. Three groups: - DOC_MUTATION (4 tests): testStreamstatsGlobalWithNull, testStreamstatsGlobalWithNullBucket, testStreamstatsResetWithNull, testStreamstatsResetWithNullBucket seed state via PUT+DELETE. Doc-level DELETE is unsupported on the parquet store, and same-_id PUT is append-only, so the leaked doc inflated the row counts of every sibling test that reads the shared index. Gated with the same DOC_MUTATION capability the three existing mutation tests already carry. - CHAINED_STREAMSTATS_BY (4 tests): chaining two streamstats where an upstream stage partitions `by` a group emits a ROW_NUMBER() sequence column from each stage; the Substrait converter names both physical columns identically, so the stacked schema has a duplicate/ambiguous field name (500) or, for chained window streamstats, non-deterministic values. The Calcite logical plan is correct; the alias is lost in Substrait conversion. Fails single- and multi-shard. - STREAMSTATS_SORT_NOT_HONORED (1 test): testStreamstatsAndSort. The window is computed over the backend scan order, ignoring a preceding `| sort` (the OVER clause carries no explicit ORDER BY), so the per-row aggregates diverge from the v2/Calcite path. Pass rate on the single-shard analytics route, CalciteStreamstatsCommandIT: | metric | before | after | |-----------|--------|-------| | tests run | 47 | 42 | | failures | 12 | 0 | | skipped | 3 | 7 | (The before-failures count is inflated by the DOC_MUTATION leak described above; the four leaking tests plus their downstream row-count victims all clear once gated.) v2 route (:integTest): 47 run, 0 failed, 0 skipped — gates are no-ops off the analytics route. Twelve further tests fail only on the multi-shard route (they pass single-shard) due to cross-shard fragment-order non-determinism in the streamstats window gather; that is an engine-side gap and is left unchanged here rather than gated, to avoid skipping passing tests on the single-shard route. Signed-off-by: Kai Huang <ahkcs@amazon.com>
… commit to unblock CI (opensearch-project#5583) Signed-off-by: Eric Wei <mengwei.eric@gmail.com>
…patibility (opensearch-project#5584) * test(integ-test): fix engine-agnostic IT queries Stabilize non-deterministic GROUP BY results with ORDER BY, normalize LENGTH() case, and use ANSI positional GROUP BY. Correctness fixes that apply regardless of execution engine. Signed-off-by: Chen Dai <daichen@amazon.com> * test(integ-test): relax schema matcher for analytics engine Signed-off-by: Chen Dai <daichen@amazon.com> --------- Signed-off-by: Chen Dai <daichen@amazon.com>
…alias field (opensearch-project#5577) Fixes opensearch-project#5533. When `@timestamp` is defined as a field-type alias in the index mapping, multisearch queries threw: ClassCastException: RelCompositeTrait cannot be cast to RelCollation Root cause: `reIndexCollations()` in `CalciteLogicalIndexScan` and `pushDownSort()` in `AbstractCalciteIndexScan` both called `RelTraitSet.plus()` to update the collation trait on a scan node. `plus()` *composes* traits — if the trait set already contains a `RelCollation`, it merges the old and new collations into a `RelCompositeTrait`. Calcite's `RelTraitSet.getCollation()` then does an unchecked cast `(RelCollation) getTrait(...)` which fails at runtime for `RelCompositeTrait`. The `@timestamp` alias path specifically triggers this because `wrapProjectForAliasFields()` adds a project on top of each sub-scan which is later pushed back down via `pushDownProject()`. `pushDownProject()` calls `reIndexCollations()` to remap field indices inside an existing collation — but re-using `plus()` here composes the existing sort collation with the re-indexed one, producing the bad composite. Fix: use `RelTraitSet.replace()` in both locations. `replace()` substitutes the collation trait in-place regardless of what was there before, which is the correct semantics for "this scan is now sorted by these columns". Added a regression IT (`testMultisearchWithTimestampAliasFieldDoesNotThrow`) that runs a multisearch against `TEST_INDEX_ALIAS`, whose mapping defines `@timestamp` as an alias for `original_date`. Signed-off-by: Radhakrishnan Pachyappan <gingeekrishna@gmail.com>
…h-project#5587) Window functions outside WINDOW_FUNC_MAPPING (e.g. RANK) used to escape the AE route as HTTP 500 because the throw site emitted a raw UnsupportedOperationException, which UnifiedQueryPlanner rethrows unchanged. Switching to CalciteUnsupportedException lets the existing 4xx wrapper added in opensearch-project#5569 normalize it to SemanticCheckException. Repro: SELECT RegionID, COUNT(*) AS cnt, RANK() OVER (ORDER BY COUNT(*) DESC) AS rnk FROM clickbench GROUP BY RegionID LIMIT 5 Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
…arch-project#5592) This PR fixes SQL window functions used with ORDER BY / LIMIT, which produced wrong plans for Analytics Engine). Because fixing the shared AstBuilder directly would impact the SQL V2 engine (V2 requires the top operator to be a Project), the fix is implemented in the extended AST builder of the unified query API only. Signed-off-by: Chen Dai <daichen@amazon.com>
…rch-project#5581) SQL plugin routing fix: - RestUnifiedQueryAction.isAnalyticsIndex() now splits comma-separated index names and checks each independently. Routes to analytics engine only if ALL indices are composite. Previously, the joined string 'idx1,idx2' was looked up as a single index in cluster metadata, causing multi-index queries to fall through to the legacy pipeline. Regression tests: - testPPLMultiIndexDeniedWhenSecondIndexUnauthorized - testPPLMultiIndexDeniedWithBackticksAuthorizedFirst - testPPLMultiIndexDeniedWithUnauthorizedFirst - testPPLMultiIndexAllowedWhenAllAuthorized Also fixes plugin install order (composite-engine before backends). Signed-off-by: Finnegan Carroll <carrofin@amazon.com> Signed-off-by: Finn Carroll <carrofin@amazon.com>
…pensearch-project#5585) * test(integ-test): gate analytics-engine excludes via @RequiresCapability Migrate the analytics-engine IT exclusions to method/class-level @RequiresCapability annotations so each test self-skips when the analytics engine is active, instead of relying on build.gradle excludes. Add coarse capabilities (backend: vector/geopoint/identifier, untyped NULL literal, filtered aggregate; frontend: response format, pagination/cursor, prepared statement, legacy method query, query error, explain format, function type compat) and reuse existing capabilities where they fit. SQLCorrectnessIT is outside the SQLIntegTestCase hierarchy, so it calls BackendCapabilities.requireCapability directly. Signed-off-by: Chen Dai <daichen@amazon.com> * test(integ-test): skip index cleanup when client is null Signed-off-by: Chen Dai <daichen@amazon.com> --------- Signed-off-by: Chen Dai <daichen@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
…ch (opensearch-project#5589) A vectorSearch() call with more arguments than its resolved signature declares (for example a duplicate named argument such as table='x', table='x') crashed with an unchecked IndexOutOfBoundsException surfaced as HTTP 500. The resolver returns a fixed-arity signature regardless of how many arguments it was called with, so castArguments looped over the supplied arguments while indexing into the shorter resolved-type list and ran off the end. Guard the argument count against the resolved signature in castArguments and throw an ExpressionEvaluationException, which maps to a clean 400, before any indexing. This protects every custom function resolver, not only vectorSearch(). Signed-off-by: Eric Wei <mengwei.eric@gmail.com>
…earch-project#5602) Introduce plugins.query.max_expression_depth (default 1000; 0 to disable) to bound expression nesting depth during AST building, improving robustness for very large or deeply nested SQL/PPL queries. Signed-off-by: Chen Dai <daichen@amazon.com>
* Add json_tree explain format Signed-off-by: Simeon Widdis <sawiddis@amazon.com> * code review updates Signed-off-by: Simeon Widdis <sawiddis@amazon.com> * revert all those explain tests Signed-off-by: Simeon Widdis <sawiddis@amazon.com> * revert explain behavior to old behavior outside of json_tree path Signed-off-by: Simeon Widdis <sawiddis@amazon.com> * revert doctest updates Signed-off-by: Simeon Widdis <sawiddis@amazon.com> * when pushdown is disabled, there's no sourcebuilder Signed-off-by: Simeon Widdis <sawiddis@amazon.com> --------- Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
* Add PPL timewrap command for time-period comparison Implement the timewrap command that reshapes timechart output by wrapping each time period into a separate data series, enabling day-over-day, week-over-week, and other recurring interval comparisons. Signed-off-by: Jialiang Li <jialiang.li@hey.com> Signed-off-by: Kai Huang <ahkcs@amazon.com> * Update metadata.rst doctest for timewrap_test index Add timewrap_test to SHOW TABLES expected output (25 tables). Signed-off-by: Jialiang Li <jialiang.li@hey.com> Signed-off-by: Kai Huang <ahkcs@amazon.com> * Refactor timewrap: extract TimewrapUtils, add precise calendar arithmetic - Extract all timewrap helper methods to TimewrapUtils.java in calcite/utils/ - Add precise EXTRACT-based period computation for month/quarter/year - Add cumDaysBeforeMonth with leap year CASE expression for precise quarter offset - Month/quarter/year period assignment now uses calendar arithmetic instead of approximate fixed-length conversions Signed-off-by: Jialiang Li <jialiang.li@hey.com> Signed-off-by: Kai Huang <ahkcs@amazon.com> * Replace Calcite PIVOT with post-processing pivot, add series parameter - Remove Calcite PIVOT from timewrap: no more MAX_PERIODS limit or crash risk - Add post-processing pivot in execution engine: dynamically builds columns from actual data using HashMap grouping - Add series parameter: series=relative (default), series=short (s0, s1) - Add series=exact grammar support (falls back to short at runtime) - Add time_format grammar support for series=exact - Benchmark: 1000 period columns in ~50ms (Calcite PIVOT crashed at 1000) - 32 IT tests, all using verifySchema + verifyDataRows Signed-off-by: Jialiang Li <jialiang.li@hey.com> Signed-off-by: Kai Huang <ahkcs@amazon.com> * Support timewrap on the analytics-engine route The timewrap pivot (turning the unpivoted [display_ts, value, __base_offset__, __period__] rows into Splunk-style period columns) was done as post-processing in OpenSearchExecutionEngine.buildResultSet, gated on CalcitePlanContext thread-locals. The analytics-engine route executes the RelNode via AnalyticsExecutionEngine and never reaches that code, so timewrap queries came back unpivoted (CalciteTimewrapCommandIT 0/32 on the analytics route). Extract the pivot into a shared core helper (TimewrapPivot) and call it from both execution engines so they produce identical output. AnalyticsExecutionEngine captures the timewrap signals at execute() entry via TimewrapSignals because the result callback runs on a different worker thread than the planning thread that set the thread-locals. CalciteTimewrapCommandIT: 32/32 on both the Calcite/v2 and analytics-engine routes. Signed-off-by: Kai Huang <ahkcs@amazon.com> * Address timewrap review: thread-local leak, anonymizer, floor-divide, docs, unit tests Fixes from PR review: - Thread-local leak: visitTimewrap sets timewrap signals on CalcitePlanContext thread-locals during planning, but they were only cleared on the execute success path. explain and exception-after-planning paths leaked the signals onto the pooled worker thread, so the next query was wrongly treated as timewrap. Centralize clearing in CalcitePlanContext.clearTimewrapSignals(), called from CalcitePlanContext.run()'s finally (v2 path) and from RestUnifiedQueryAction's finally (analytics path, which doesn't use run()). Both engines' existing clears now route through the one helper. - Anonymizer: add visitTimewrap to PPLQueryDataAnonymizer so anonymized query logs include the timewrap command instead of silently dropping the pipe segment. span magnitude is masked; align/series are constrained keywords. - align=now off-by-one: baseOffset used integer DIVIDE (truncates toward zero), giving wrong period labels when the reference is below maxEpoch (future-dated data under align=now). Use FLOOR(double-divide) for true floor division. - Docs/dead code: remove the unimplemented "max 20 period columns" claim from timewrap.md and the unused MAX_PERIODS constant (the pivot is intentionally unbounded). - Tests: add CalcitePPLTimewrapTest (verifyLogical + verifyPPLToSparkSQL) per the PPL-command checklist, and a timewrap case in PPLQueryDataAnonymizerTest. CalciteTimewrapCommandIT: 32/32 on both the Calcite/v2 and analytics routes. Signed-off-by: Kai Huang <ahkcs@amazon.com> * Address timewrap review: remove dead time_format plumbing, optimize pivot Follow-up cleanups from PR review: - series=exact: keep the documented fallback to short "s<N>" naming, but remove the write-only CalcitePlanContext.timewrapTimeFormat thread-local (set in visitTimewrap and cleared in clearTimewrapSignals, but never read). The AST Timewrap.timeFormat field is retained for when exact formatting is implemented. Collapse the identical short/exact switch arms with a comment. - TimewrapPivot: precompute each period's display name once into a Map<Long,String> instead of re-running split/parseInt/switch inside the per-row loop (was O(rows x valueCols)). Collapse the unreachable two-pass column-index scan into one — visitTimewrap always emits the bookkeeping columns last, so the fallback scan never ran. CalciteTimewrapCommandIT: 32/32 on both the Calcite/v2 and analytics routes. Signed-off-by: Kai Huang <ahkcs@amazon.com> * Add CalciteExplainIT explain-plan tests for timewrap Two explain-plan tests following the existing CalciteExplainIT pattern (explainQueryYaml + loadExpectedPlan + assertYamlEqualsIgnoreId): - testExplainTimewrap: fixed-length unit (1day), epoch-based arithmetic path. - testExplainTimewrapMonth: variable-length unit (1month), EXTRACT-based calendar arithmetic path. Both pin the align=end reference with a WHERE @timestamp <= upper bound so the base_offset literal is deterministic across runs. Signed-off-by: Kai Huang <ahkcs@amazon.com> * Add no-pushdown expected plans for timewrap explain tests CalciteNoPushdownIT re-runs CalciteExplainIT with pushdown disabled, which loads expected plans from expectedOutput/calcite_no_pushdown/ instead of expectedOutput/calcite/. The two timewrap explain tests were missing their no-pushdown variants, causing "resource ... not found" failures in CI. Logical plans match the pushdown variants; the physical plans differ (EnumerableLimit/EnumerableSort + full index scan, no pushed-down aggregation). Signed-off-by: Kai Huang <ahkcs@amazon.com> * Reject variable-length units in spanToSeconds spanToSeconds is only used on the fixed-length timewrap path (s/m/h/d/w), where the second count is exact. The M/q/y arms returned approximate 30/91/365-day values that were never used for wrapping (those units go through the calendar arithmetic path with exact leap-year handling). Throw instead so the approximation can never be silently consumed. Signed-off-by: Kai Huang <ahkcs@amazon.com> * Narrow timestamp-parse catches to DateTimeParseException Both LocalDateTime.parse and Instant.parse throw only DateTimeParseException on bad input; catch that specific type instead of Exception so genuine programming errors surface. Signed-off-by: Kai Huang <ahkcs@amazon.com> * Add leak-guard test for timewrap pivot signals Asserts that after a timewrap query runs through CalcitePlanContext.run, the pivot thread-locals are cleared so the next non-timewrap query on the same pooled thread is not wrongly pivoted (no __base_offset__/__period__ artifacts). Verified the test fails if run()'s clearTimewrapSignals guard is removed. Signed-off-by: Kai Huang <ahkcs@amazon.com> --------- Signed-off-by: Jialiang Li <jialiang.li@hey.com> Signed-off-by: Kai Huang <ahkcs@amazon.com>
…earch-project#5603) PPL integer arithmetic (`+`, `-`, `*` and the named add/subtract/multiply functions) inferred `SMALLINT op SMALLINT -> SMALLINT` (and likewise for TINYINT) because the operators were registered with Calcite's stock `SqlStdOperatorTable.PLUS/MINUS/MULTIPLY`, whose return-type inference (`ReturnTypes.NULLABLE_SUM` / `PRODUCT_NULLABLE`) falls through to `LEAST_RESTRICTIVE` for non-decimal integers. As a result the product/sum of two narrow-integer columns overflowed the inferred type on every backend, just differently: - DataFusion / analytics-engine: silently wraps the i16 result, so e.g. `eval area = ResolutionWidth * ResolutionHeight | where area > 2000000` returned 0 rows instead of the matching row. - Calcite Enumerable engine: throws `ArithmeticException: value out of range`. - v2 legacy engine: `ExprShortValue` narrows via `shortValue()`, wrapping. Fix in the SQL-plugin lowering so all backends are corrected at once: widen the operands (byte/short -> INTEGER, any int/long -> BIGINT) before applying the operator. Casting the operands rather than only relabelling the result type is required, otherwise DataFusion still computes the narrow multiply and wraps before any outer cast. Non-integral operands (float/double/decimal/ datetime/mixed) are left untouched and defer to Calcite's default inference. The string-concat `ADD` variant and the DATETIME-DATETIME `SUBTRACT` variant are unchanged. mvindex's internal array-index arithmetic now uses the raw Calcite PLUS/MINUS operators so array indices stay INTEGER for ITEM/ARRAY_SLICE codegen (the widened result would otherwise be rejected as a long index). Note: this changes user-visible result column types for integer arithmetic (int-operand expressions now report bigint), which is the intended trade-off for overflow-safe, backend-consistent results. Adds CalcitePPLBuiltinFunctionIT coverage for the short->int and int->bigint widening tiers and updates the affected logical-plan / Spark-SQL snapshots. Signed-off-by: Kai Huang <ahkcs@amazon.com>
…roject#5586) - Replace old backport workflow with reusable workflow from opensearch-build - Remove obsolete backport-related workflows Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
…arch-project#5618) Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
…le alias collision (opensearch-project#5593) * fix(dedup): use Map<String,List<String>> in fieldNameMapping to handle alias collision (opensearch-project#5197) When `rename` and `eval` column-ref both resolve to the same source field (e.g. `eval nm2 = name | rename name as nm`), the previous Map<String,String> approach silently dropped one mapping on collision. This commit implements the fix from scratch (PR opensearch-project#5192 was closed unmerged): * TopHitsParser: add an optional `Map<String, List<String>> fieldNameMapping` field and a new 4-arg constructor; the 3-arg constructor delegates with null (no-op). `applyFieldNameMapping()` copies the source-field value to every output alias and removes the original key only when it is not itself an expected output name. * AggregateAnalyzer: in the LITERAL_AGG (dedup) branch, build fieldNameMapping by iterating over the projection args; pass it to TopHitsParser when non-empty. * Unit tests (OpenSearchAggregationResponseParserTest): - `top_hits_field_name_mapping_single_rename_should_pass` – regression for opensearch-project#5150 - `top_hits_field_name_mapping_collision_should_duplicate_value` – regression for opensearch-project#5197 * Integration tests (CalcitePPLDedupIT): - `testDedupWithRenamedField` – dedup after rename, single alias - `testDedupWithRenamedFieldMappingCollision` – dedup after both rename and eval alias Fixes opensearch-project#5197 Signed-off-by: Radhakrishnan Pachyappan <gingeekrishna@gmail.com> * review: address PR feedback — null guard and map.get optimisation * AggregateAnalyzer: guard against a null return from inferNamedField before calling getRootName() (defensive; the method returns non-null for RexInputRef today but the null check makes the contract explicit). * TopHitsParser.applyFieldNameMapping: replace the containsKey+get double lookup with a single map.get() call; null value + absent key is distinguished via containsKey only when value is null, eliminating the redundant containsKey in the common (non-null value) path. Fixes opensearch-project#5150 Fixes opensearch-project#5197 Signed-off-by: Radhakrishnan Pachyappan <gingeekrishna@gmail.com> * test: fix testDedupWithRenamedField* expected rows for category Y The test data (duplication_nullable.json) has category=Y rows in this order: A (id 2), A (id 3), null (id 8), A (id 12), B (id 15). 'dedup 1 category' keeps the FIRST occurrence per category, which has name=A for category Y, not B. Update expected rows: rows("Y","B") -> rows("Y","A") and rows("Y","B","B") -> rows("Y","A","A"). Signed-off-by: RadhaKrishnan Rajendran <gingeekrishna@gmail.com> Signed-off-by: Radhakrishnan Pachyappan <gingeekrishna@gmail.com> --------- Signed-off-by: Radhakrishnan Pachyappan <gingeekrishna@gmail.com> Signed-off-by: RadhaKrishnan Rajendran <gingeekrishna@gmail.com>
…ject#5619) * Bump Apache Calcite 1.41.0 -> 1.42.0 (CVE-2026-46718) Calcite 1.5.0 through 1.41.x are affected by CVE-2026-46718 (GHSA-c2rv-hwqm-wjpg, CWE-470 unsafe reflection), fixed in 1.42.0. Bump calcite-core, calcite-linq4j, calcite-babel, and calcite-testkit to 1.42.0. This transitively bumps avatica-core 1.27.0 -> 1.28.0. In 1.42.0, RelDataTypeSystemImpl.getMaxNumericPrecision() and getMaxNumericScale() became final; they now delegate to getMaxPrecision(DECIMAL)/getMaxScale(DECIMAL). OpenSearchTypeSystem overrode the former pair to keep Spark-aligned DECIMAL precision/scale of 38, so move those values into getMaxPrecision/getMaxScale for the DECIMAL case to preserve identical behavior. Update PPL Calcite unit-test golden strings to match 1.42.0's cosmetic plan and Spark-SQL rendering changes (explicit literal type suffixes, self-reference alias disambiguation, MAP/ARRAY type spelling, removal of redundant parentheses, explicit no-op casts). No query semantics change. Signed-off-by: Kai Huang <ahkcs@amazon.com> * Align json-path and joou-java-6 with Calcite 1.42.0 transitive deps Calcite 1.42.0 bumps two of its runtime transitive dependencies: com.jayway.jsonpath:json-path 2.9.0 -> 2.10.0 and org.jooq:joou-java-6 0.9.4 -> 0.9.5. Modules built under the OpenSearch Gradle plugin's strict failOnVersionConflict (plugin, doc, integ, security-it, bwc) fail to resolve because core pinned json-path to 2.9.0 and the old joou 0.9.4 remained in the tree. Bump the explicit json-path pin in core to 2.10.0 and force both modules to the Calcite-1.42.0 versions in the root configurations.all block, alongside the existing transitive-conflict forces. Verified with the CI unit command (build -x integTest -x yamlRestTest -x doctest): BUILD SUCCESSFUL, no conflicts. Signed-off-by: Kai Huang <ahkcs@amazon.com> * Fix FROM_UNIXTIME method resolution for boxed numeric operands Calcite 1.42.0 passes a nullable numeric operand to FROM_UNIXTIME as a boxed java.lang.Double, so method resolution could not match the primitive fromUnixTime(double) overload and codegen failed with NoSuchMethodException. This surfaced as a 500 SQLException on bin span queries over timestamp fields (e.g. bin @timestamp span=1h). Box the numeric operand and accept it as Number, mirroring SecToTimeFunction, so resolution succeeds for both primitive and boxed inputs. Signed-off-by: Kai Huang <ahkcs@amazon.com> * Fix tostring() method resolution for boxed numeric operands Same Calcite 1.42.0 boxing change as FROM_UNIXTIME: a nullable numeric operand (e.g. a BIGINT field) now reaches tostring() boxed as java.lang.Long, which matched none of the primitive double/int overloads and failed codegen with NoSuchMethodException (500 SQLException on tostring(<numeric field>, <format>), e.g. tostring(balance, 'hex')). Box the numeric operand and resolve to a single Number overload, preserving BigDecimal precision by passing DECIMAL values through unchanged. String operands keep their existing path. Signed-off-by: Kai Huang <ahkcs@amazon.com> * Update Calcite 1.42.0 explain-plan golden files Calcite 1.42.0 changes cosmetic plan rendering and some cost-based physical-plan choices. Regenerate the CalciteExplainIT and CalcitePPLClickBenchIT expected outputs (pushdown and no-pushdown variants) to match. Changes are limited to expected-output resources: - String literals now render with a type annotation ('x' -> 'x':VARCHAR). - Null-check operand references shift with equivalent projection indices (IS NOT NULL($t8) -> IS NOT NULL($t11)). - Anonymous projected columns render with their real alias. - streamstats reset: the planner now prefers HashJoin/TopK over MergeJoin + Sort + Limit. The pushdown request bodies are unchanged and CalciteStreamstatsCommandIT result assertions still pass, so query semantics are preserved. Signed-off-by: Kai Huang <ahkcs@amazon.com> * Centralize Calcite dependency version Signed-off-by: Kai Huang <ahkcs@amazon.com> --------- Signed-off-by: Kai Huang <ahkcs@amazon.com>
Signed-off-by: Kai Huang <ahkcs@amazon.com>
* [Feature] Add PPL `rest` command (Calcite system row source) Add a leading `rest <endpoint>` command that exposes a curated, read-only, fixed-schema set of in-cluster management endpoints as a PPL table, modeled as a system row source bridged through visitRelation (the same seam as describe and the system-index family), so it runs on the Calcite engine without the unsupported table-function path. - Grammar/AST: REST/TIMEOUT tokens, restCommand rule, RestRelation, AstBuilder visit, query anonymizer. - Execution: RestSourceTable -> CalciteLogicalRestScan / CalciteEnumerableRestScan; RestEndpointRegistry (read-only allow-list + fixed schema + accepted args); RestEnumerator/RestRequest dispatch via OpenSearchClient (NodeClient in-cluster, RestClient standalone). - 9 endpoints: cluster health/state/settings, cat indices/nodes/cluster_manager/ plugins/shards, resolve/index. - Output shaping: numeric type normalization, id-to-name resolution, role-name expansion, structural flattening, graceful null degrade. - Args: count caps emitted rows; timeout reserved but rejected with 400; get-args applied server-side with per-arg value validation (local on health, health on cat/indices, expand_wildcards on resolve/index). Undeclared arg or out-of-domain value is rejected with a 400. level and include_defaults are deferred to a later release; flat_settings is dropped as redundant. - Error handling: blank endpoint, negative count, disallowed arg, and uncoercible value all surface clean 400s rather than 500s. Tests: CalcitePPLRestIT 25/25, RestEndpointRegistryTest 16, RestSourceTableTest 10. Signed-off-by: Louis Chu <clingzhi@amazon.com> * Align rest '/_cluster/settings' redaction with native endpoint; CI fixes; comment cleanup - /_cluster/settings: run the persistent and transient tiers through the node SettingsFilter (published via RestSettingsFilterHolder from SQLPlugin#getRestHandlers) so Property.Filtered and plugin-registered pattern settings are redacted exactly as the native GET /_cluster/settings endpoint. Remove the dead secretFields column-filter, which was the wrong shape for the (setting, value, tier) rows. - Parser: add TIMEOUT to searchableKeyWord so a bare 'timeout' term still matches searchLiteral. - coerce(): narrow the catch to IllegalArgumentException | ClassCastException; add empty-string guards in toNumber/toBoolean. - spotlessApply formatting; drop outdated and redundant comments. Tests: RestEndpointRegistryTest, RestSourceTableTest, OpenSearchNodeClientClusterSettingsFilterTest green. Signed-off-by: Louis Chu <clingzhi@amazon.com> * Address rest command bot-review findings; register rest doctest - clusterSettings: fail closed (throw IllegalStateException) when the node SettingsFilter is unavailable, instead of returning unredacted settings - collectSettings: handle list-type settings via getAsList fallback - decodeRestSpec: reject a blank/missing endpoint with a clear error - docs: correct rest.md allow-list table (9 endpoints + accepted args), quote endpoint literals, fix timeout/get-arg descriptions, add security note - register docs/user/ppl/cmd/rest.md in docs/category.json (deterministic single-node examples: number_of_nodes=1, cluster_manager count=1) Signed-off-by: Louis Chu <clingzhi@amazon.com> * Harden decodeRestSpec: reject non-rest-source tokens with a clear error decodeRestSpec is only ever called behind an isRestSource gate today, but as a public decoder it must not assume its precondition. Without the guard a malformed token would surface an opaque StringIndexOutOfBoundsException from substring; now it throws a clear IllegalArgumentException instead. Addresses the PR opensearch-project#5599 Code Suggestions finding (importance 8). Signed-off-by: Louis Chu <clingzhi@amazon.com> * Fix rest explain IT (JSON payload) and harden cluster-settings/state fetch - CalciteExplainIT.explainRestCommand: single-quote the endpoint literal; the explain harness inlines the query into a JSON body without escaping, so a double-quoted literal produced an invalid payload and a 400 (integration CI failure). - OpenSearchNodeClient.clusterSettings: resolve the SettingsFilter and fail closed BEFORE fetching cluster state, so settings are never read into memory when the redaction filter is unavailable. - OpenSearchRestClient.clusterState: narrow filter_path to nodes.*.name so node IPs/attributes are not over-fetched; manager-name resolution is preserved. Signed-off-by: Louis Chu <clingzhi@amazon.com> * Rest command: analytics-engine coexistence IT + hardened source-token decode - AnalyticsEngineCompatIT: assert | rest '/_cluster/health' behaves identically with the analytics engine enabled (rest is never routed to DataFusion). - SystemIndexUtils.fromHex: reject an odd-length hex body so a crafted source name that passes the isRestSource suffix check fails clearly rather than silently dropping the trailing half-byte. Signed-off-by: Louis Chu <clingzhi@amazon.com> * [Bugfix] Keep rest command on Calcite path under cluster-composite On a cluster started with cluster.pluggable.dataformat=composite, RestUnifiedQueryAction.isAnalyticsIndex() routed every non-system-catalog PPL query to the analytics engine. The rest command's reserved in-cluster source (REST...__REST_SOURCE) has no backing index and only resolves on the Calcite path, so it was routed to DataFusion and failed with "Table 'REST...__REST_SOURCE' not found". Fix: exclude isRestSource(name) alongside isSystemCatalog(name) so a rest source falls back to the default (Calcite) pipeline and is never routed to the analytics engine. - RestUnifiedQueryActionTest: unit repro under cluster-composite. - integ-test analyticsEngineCompat testcluster set composite-default so the existing rest coexistence IT exercises this routing exclusion. Signed-off-by: Louis Chu <clingzhi@amazon.com> * [Refactor] Unify system-index and rest scans behind one catalog table Collapse the two near-duplicate Calcite scan hierarchies -- SHOW/DESCRIBE system tables and the rest command -- into one generic OpenSearchCatalogTable whose per-endpoint behavior is supplied by a pluggable CatalogSource. - OpenSearchSystemIndex + RestSourceTable -> one OpenSearchCatalogTable backed by SystemIndexCatalogSource / RestCatalogSource. - Two Abstract/Logical/Enumerable scans + two enumerators + two converter rules -> AbstractCalciteCatalogScan / CalciteLogicalCatalogScan / CalciteEnumerableCatalogScan (+ rest-only CalciteScannableCatalogScan) / OpenSearchCatalogEnumerator / EnumerableCatalogScanRule. - Concerns stay per-source: system tables keep the real V2 implement() path; rest is Calcite-only (implement() throws) and opts into Scannable for the collect short-circuit. No behavior change: dispatch, schemas, V2 support, and the Scannable marker are preserved; this is pure de-duplication of the Calcite scan plumbing. Verified: opensearch compileJava/compileTestJava green; ppl and integ-test test-compile green; affected unit tests pass. Signed-off-by: Louis Chu <clingzhi@amazon.com> * [Feature] Add rest command response redaction and endpoint allow-list Two dynamic cluster settings gate the rest command per deployment: - plugins.ppl.rest.redaction.enabled (default false): mask network identifiers in _cat/* cells and availability-zone names in _cluster/settings values. - plugins.ppl.rest.allowed_endpoints (default all): restrict which endpoints are served; an empty list disables the rest command. Both default to open-source parity: all endpoints served, no masking. Signed-off-by: Louis Chu <clingzhi@amazon.com> * [Refactor] Remove unused REST/TIMEOUT rules from shared language grammar The shared language-grammar carried REST and TIMEOUT lexer tokens and the restCommand/restArgument parser rules with no consumer: async-query-core has no rest visitor, and the rest command grammar lives in the ppl module. Remove the dead rules. Signed-off-by: Louis Chu <clingzhi@amazon.com> * [Test] Add rest command security integration tests Verify the rest command is subject to the security plugin fine grained access control: a caller without cluster:monitor privilege is denied the cat and cluster endpoints, a caller holding the privilege can run them, and the resolve index endpoint is filtered to the caller authorized indices. Test indices are created idempotently, and denials are asserted by the security denial reason in the response body because a denied action on the Calcite only rest path surfaces as a wrapped error carrying that reason. Calcite fallback is disabled so the denial reason is not replaced by an unsupported command error. Signed-off-by: Louis Chu <clingzhi@amazon.com> * [Bugfix] Make rest redaction and allow-list settings node-level plugins.ppl.rest.redaction.enabled and plugins.ppl.rest.allowed_endpoints were dynamic cluster settings, so they could be changed at runtime through _cluster/settings or the _plugins/_query/settings endpoint. On a managed deployment that let a caller disable redaction or widen the allow-list an operator had configured. Drop Setting.Property.Dynamic so both are node-level settings set in the node config; the engine rejects runtime updates on both paths. Register them without an update consumer and read the node-configured value. Signed-off-by: Louis Chu <clingzhi@amazon.com> * Enhance redaction logic Signed-off-by: Louis Chu <lingzhichu.clz@gmail.com> * [Change] Disable all rest endpoints by default (empty allow-list) Flip plugins.ppl.rest.allowed_endpoints default from ["*"] to empty so open source ships with the rest command closed. Deployments opt specific endpoints in via the setting (AOS enables the ones it supports; AOSS leaves it empty and stays disabled). Enforcement already treats an empty or missing list as disabled. Opt the integration-test clusters into all endpoints so the rest ITs still exercise the enabled path. Signed-off-by: Louis Chu <lingzhichu.clz@gmail.com> --------- Signed-off-by: Louis Chu <clingzhi@amazon.com> Signed-off-by: Louis Chu <lingzhichu.clz@gmail.com>
Signed-off-by: Kai Huang <ahkcs@amazon.com>
* Implement PPL foreach command
Signed-off-by: Songkan Tang <songkant@amazon.com>
* Complete PPL foreach collection modes
Signed-off-by: Songkan Tang <songkant@amazon.com>
* Extract foreach planner constants
Signed-off-by: Songkan Tang <songkant@amazon.com>
* Complete foreach collection type inference
Signed-off-by: Songkan Tang <songkant@amazon.com>
* Refactor foreach command: reuse type system, extract planner, remove special cases
- Extract ~400 lines of foreach planning from CalciteRelNodeVisitor into
a dedicated ForeachPlanner class.
- Replace string-based type plumbing (SqlTypeName names smuggled through
ForeachBinding and reparsed at runtime) with RelDataType carried on the
binding; foreach_pair_item now takes (pair, index) only and the call is
built with the plan-time type directly.
- Collapse ForeachBindingType {PAIR_ITEM, PAIR_ITER, PAIR_EXTRA, LAMBDA}
into a single PAIR_SLOT; drop the unused LAMBDA variant.
- Replace the AST arithmetic-scan heuristic for json_array element types
with operand type inspection (json_array call) / plan-time JSON parse
(string literal), using SqlTypeFamily.
- Remove the reduce-arg0 special case in CalciteRexNodeVisitor: collection
bindings are now staged on CalcitePlanContext and only activate inside
cloned lambda contexts, so non-lambda reduce args resolve against the
row naturally.
- Foreach.Mode enum replaces stringly-typed mode comparisons; AstBuilder
parses options/targets in a single pass without double-visiting
expressions.
- Delete dead code: FOREACH_TRANSFORM_* constants, FOREACH_PAIR_ITEM
registry entry, validateHomogeneousJsonArrayArguments wrapper, duplicate
collectionExpressionForMode branches.
Signed-off-by: Songkan Tang <songkant@amazon.com>
* Narrow foreachTarget grammar to fix ANTLR error-recovery regression
foreachTarget's logicalExpression alternative let ANTLR consider the
foreach rule during error recovery of unrelated malformed queries,
changing expected-token sets in syntax error messages (caught by
UnifiedRelevanceSearchTest#testMatchMissingArguments: 'match(' with a
missing field started reporting 26 candidate tokens instead of the
expected ','). foreach only ever needs a function call (json_array),
a string literal (JSON array text), or a field/wildcard as its target,
so accept exactly those instead of the whole expression grammar.
Signed-off-by: Songkan Tang <songkant@amazon.com>
* Assert full logical plans in foreach collection-mode unit tests
Replace assertNotNull with verifyLogical so the tests pin the generated
reduce/foreach_pair_collection/foreach_pair_item structure, placeholder
slot indices, and json_array element-type inference.
Signed-off-by: Songkan Tang <songkant@amazon.com>
* Restore usage-based element type inference for field-backed JSON arrays
The refactor's jsonElementType defaulted opaque expressions (an index
field holding JSON text - the primary Splunk use of json_array mode) to
VARCHAR, which broke numeric aggregation over such fields: reduce
resolved to [ARRAY<VARCHAR>, DOUBLE, DOUBLE] and failed. Splunk returns
60 for a field holding "[10,20,30]" summed via foreach; the original
branch matched that by inferring element type from usage.
Bring back the usage scan for the opaque case only: if the item
placeholder is consumed by arithmetic the elements are DOUBLE, else
VARCHAR. json_array() calls and string literals keep the plan-time
content inspection.
New coverage:
- CalcitePPLForeachTest: plan assertions for field-backed json_array
with numeric and string usage
- ForeachFieldJsonIT: field holding "[10,20,30]" sums to 60 (Splunk
parity), string-content field concats, and native OpenSearch array
fields (long field holding [1,2,3]) documented as rejected - the
mapping types them as scalar BIGINT at plan time
Signed-off-by: Songkan Tang <songkant@amazon.com>
* Cover nested-field multivalue iteration in ForeachFieldJsonIT
Nested-typed fields map to ARRAY<ANY> at plan time so multivalue mode
accepts and iterates them (verified: counting elements of a 2-element
nested array returns 2). Pins the third field-backed collection shape
alongside JSON-text fields (supported) and native scalar-mapped arrays
(rejected).
Signed-off-by: Songkan Tang <songkant@amazon.com>
* Pin cross-feed behavior of foreach collection modes in IT
Splunk silently no-ops when a mode is fed the wrong collection shape
(verified against Splunk 10.4.0). Our behavior intentionally differs
and these tests document it:
- json_array mode fed a real array iterates it (total=6) - more
permissive than Splunk's no-op
- multivalue mode fed a JSON-text field fails at plan time with
SemanticCheckException - louder than Splunk's no-op
Signed-off-by: Songkan Tang <songkant@amazon.com>
* Add user documentation for PPL foreach command
Follows the structure of other command docs (timewrap, eval): syntax,
parameters, placeholder table, notes on collection-mode accumulator
semantics and type inference, and six runnable examples registered in
docs/category.json for doctest.
All example outputs verified against a live docTestCluster loaded with
the doctest accounts dataset.
Signed-off-by: Songkan Tang <songkant@amazon.com>
* Complete new-command checklist: v2 Analyzer guard and query anonymization
- Analyzer.visitForeach throws the standard only-for-Calcite
UnsupportedOperationException instead of leaving the v2 path
undefined
- PPLQueryDataAnonymizer.visitForeach renders the command with mode,
masked option values, masked targets (collection targets can embed
literals such as JSON array strings), and anonymized eval clauses;
ForeachPlaceholder masks like a column reference
UT: AnalyzerTest legacy-engine rejection; anonymizer coverage for
multifield, multivalue-with-options, and json_array-with-literal-target
(122/122 anonymizer tests pass)
Signed-off-by: Songkan Tang <songkant@amazon.com>
* Use Locale.ROOT in testNoMv explain-plan case folding
CI runs with randomized locales; under az-Cyrl the default-locale
toLowerCase turns ARRAY_JOIN into array_joın (dotless i), so the
contains("array_join") assertions fail. Reproduced locally with
-Dtests.seed=FAD995E7580F1AF -Dtests.locale=az-Cyrl and verified fixed.
Pre-existing bug in these tests (added by the timewrap PR), surfaced on
this PR's CI run by locale randomization.
Signed-off-by: Songkan Tang <songkant@amazon.com>
* Address foreach review feedback and Splunk semantics
Signed-off-by: Songkan Tang <songkant@amazon.com>
* Cover foreach JSON numeric inference in IT
Signed-off-by: Songkan Tang <songkant@amazon.com>
---------
Signed-off-by: Songkan Tang <songkant@amazon.com>
…opensearch-project#5164) (opensearch-project#5604) * Detect long (BIGINT) arithmetic overflow instead of silently wrapping PPL/SQL long arithmetic (+, -, *) in eval/SELECT expressions silently wrapped on overflow in the Calcite engine (e.g. long_field * 999...9 wrapped to a negative value with HTTP 200). SqlStdOperatorTable.PLUS/MINUS/MULTIPLY generate plain Java +/-/* in the Enumerable code path, which wrap. Rewrite long +/-/* to their overflow-checked variants (CHECKED_PLUS / CHECKED_MINUS / CHECKED_MULTIPLY), which generate Math.addExact etc. and throw ArithmeticException on overflow. The exception is caught in QueryService and surfaced as a 4xx client error instead of wrapping or falling back to V2. Scoped to BIGINT operands only: narrower integer arithmetic (byte/short/int) is widened to a type that cannot overflow before this rewrite runs (PPLFuncImpTable promotes byte/short to INT and any int/long to BIGINT for +/-/*), so long — which has no wider integer type — is the sole remaining overflow case on the Calcite engine. Float/double/decimal follow IEEE 754 / decimal semantics and have no CHECKED_* runtime, so they are left untouched. The rewrite runs after the analytics-engine fork, so only the Calcite path is affected (the DataFusion backend handles its own overflow). The rewrite runs before pushdown so both coordinator-executed and pushed-down (script) arithmetic are checked; PPLAggregateConvertRule, OpenSearchRelOptUtil, and ExtendedRelJson recognize the CHECKED_* kinds so aggregate/sort pushdown and script serialization keep working. Adds a REST yaml test (issues/5164.yml) and updates the affected explain fixtures. Resolves opensearch-project#5164 Signed-off-by: Kai Huang <ahkcs@amazon.com> * Address checked arithmetic review feedback Signed-off-by: Kai Huang <ahkcs@amazon.com> --------- Signed-off-by: Kai Huang <ahkcs@amazon.com>
Add a cross-repository CI check that keeps the OpenSearch-Dashboards PPL lint rule 'unsupported-window-function-in-eventstats' and the SQL backend in agreement. Frontend half: a SQL-owned Node script loads the compiled OSD analyzer from an OSD checkout and asserts the rule's diagnostic counts. Backend half: a Gradle integration test sends the same queries to the live /_plugins/_ppl endpoint of the SQL plugin built from the checkout. Both halves consume one shared contract file. - integ-test/.../ppl-lint/unsupported-window-function-in-eventstats.spec.json - scripts/ppl-lint/run-frontend-contract.mjs - integ-test/.../calcite/remote/PplLintRuleValidationIT.java - .github/workflows/ppl-lint-rule-validation.yml - scripts/ppl-lint-rule-validation.sh Signed-off-by: Hanyu Wei <weihanyu@amazon.com>
Addresses shellcheck SC2006/SC2046 on the chown/su lines so actionlint runs clean. Behavior is unchanged. Signed-off-by: Hanyu Wei <weihanyu@amazon.com>
Signed-off-by: Hanyu Wei <weihanyu@amazon.com>
The OpenSearch CI container is Amazon Linux 2 (glibc 2.26), but OSD requires Node 22 whose prebuilt binary needs glibc >= 2.27. Running the Node frontend contract inside that container failed with 'GLIBC_2.27 not found'. Split into two required jobs: 'frontend' runs the OSD analyzer contract on a bare ubuntu-latest runner (modern glibc, actions/setup-node works), and 'backend' keeps the Gradle integration test in the CI container where the OpenSearch test cluster needs it. Signed-off-by: Hanyu Wei <weihanyu@amazon.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
14 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a cross-repository CI check that keeps the OpenSearch-Dashboards (OSD) PPL lint rule
unsupported-window-function-in-eventstatsand the SQL backend in agreement.scripts/ppl-lint/run-frontend-contract.mjs) loads the compiled OSD analyzer from an OSD checkout and asserts the rule's diagnostic counts.PplLintRuleValidationIT) sends the same queries to the live/_plugins/_pplendpoint of the SQL plugin built from this checkout and asserts the structured rejection (status,error.type,error.reason).mainby default).Note: this is an intra-fork PR to exercise the new GitHub Actions workflow on real runners before proposing upstream.
Check List
--signoff.