Skip to content

[Backport 2.x] Fix 1.x compatibility bug with stored Tasks (#5412) - #5415

Merged
andrross merged 1 commit into
opensearch-project:2.xfrom
andrross:backport/backport-5412-to-2.x
Nov 30, 2022
Merged

[Backport 2.x] Fix 1.x compatibility bug with stored Tasks (#5412)#5415
andrross merged 1 commit into
opensearch-project:2.xfrom
andrross:backport/backport-5412-to-2.x

Conversation

@andrross

Copy link
Copy Markdown
Member

When the new 'cancelled' field was introduced it was a miss not to increment the version number on the mapping definitions for the .tasks index. This commit fixes that oversight, as well as modifies the existing backward compatiblity test to ensure that it will catch future mistakes like this one.

Closes #5376

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@andrross
andrross marked this pull request as ready for review November 30, 2022 20:16
@andrross
andrross requested review from a team and reta as code owners November 30, 2022 20:16
@github-actions

Copy link
Copy Markdown
Contributor

Gradle Check (Jenkins) Run Completed with:

When the new 'cancelled' field was introduced it was a miss not to
increment the version number on the mapping definitions for the .tasks
index. This commit fixes that oversight, as well as modifies the
existing backward compatiblity test to ensure that it will catch future
mistakes like this one.

Closes opensearch-project#5376

Signed-off-by: Andrew Ross <andrross@amazon.com>
(cherry picked from commit 4616dfa)
Signed-off-by: Andrew Ross <andrross@amazon.com>
@andrross
andrross force-pushed the backport/backport-5412-to-2.x branch from 665491a to 8e11adf Compare November 30, 2022 20:54
@github-actions

Copy link
Copy Markdown
Contributor

Gradle Check (Jenkins) Run Completed with:

@andrross
andrross merged commit 17b9077 into opensearch-project:2.x Nov 30, 2022
@andrross
andrross deleted the backport/backport-5412-to-2.x branch December 16, 2022 18:35
RyanL1997 added a commit to RyanL1997/OpenSearch that referenced this pull request May 7, 2026
…/regexp_replace() functions

Onboards the PPL `replace` command and the `replace()` / `regexp_replace()`
eval functions to the analytics-engine route by mapping their two Calcite
lowering targets — `SqlStdOperatorTable.REPLACE` and
`SqlLibraryOperators.REGEXP_REPLACE_3` — through Substrait to DataFusion's
native `replace` and `regexp_replace` UDFs.

Same templated shape as the `fillnull` POC (opensearch-project#21472):

  ScalarFunction enum constant
    + STANDARD_PROJECT_OPS membership
    + opensearch_scalar_functions.yaml extension entry
    + ADDITIONAL_SCALAR_SIGS Calcite-op→Substrait-name bridge
    = onboarded to the analytics route.

Two scalar functions added: REPLACE (literal substring replace) and
REGEXP_REPLACE (regex replace). Both project-side only; the comparison
result of a replaced field is filtered via the existing EQUALS capability,
so no STANDARD_FILTER_OPS additions are needed.

PPL's wildcard `replace` form lowers via `WildcardUtils.convertWildcardPatternToRegex()`
to a Java-`Pattern`-compatible regex. Two flavors of Java syntax need
translation before substrait serialization, because DataFusion uses Rust's
`regex` crate which has different parsing rules:

  * `\Q…\E` quoted-literal blocks — Rust rejects `\Q` as an unrecognized
    escape sequence. The adapter expands each block to per-character
    escaped literals (semantics-preserving).
  * `$N` numeric backreferences in the replacement — Rust's replacement
    parser is identifier-greedy, so `$1_$2` is parsed as a reference to
    group named `1_` followed by `$2` (Java parses it as group 1 + literal
    underscore + group 2). The adapter wraps every numeric backreference
    in braces (`${N}`) for unambiguous Rust parsing.

Both transforms are in `RegexpReplaceAdapter` and registered against
`ScalarFunction.REGEXP_REPLACE` in `scalarFunctionAdapters()`. Calls
without `\Q` in the pattern AND without bare `$N` in the replacement pass
through unchanged.

  * `RegexpReplaceAdapterTests` — 19/19 (unquote: 9, brace: 7, dual-rewrite
    integration: 3).
  * `ReplaceCommandIT` (new self-contained QA IT, calcs dataset) — 10/10.
    Covers literal command (single + multi-pair = nested REPLACE), wildcard
    command (prefix + suffix), `replace()` and `regexp_replace()` in eval,
    full-row content checks, no-match passthrough, multi-field IN clause.
  * SQL plugin's `CalciteReplaceCommandIT` force-routed through the
    analytics-engine route via `-Dtests.analytics.{force_routing,parquet_indices}=true`
    — 21/21 in both the direct suite and the `CalciteNoPushdownIT` re-run.
    (Companion SQL plugin PR opensearch-project#5415 makes 4 column-order assertions and 1
    error-message assertion order-agnostic, mirroring the rename precedent
    from opensearch-project#5413.)

Unlike `fillnull`/`regex` where the bridge was a single one-line capability
addition, `replace`'s wildcard form exposes Java↔Rust regex syntax
divergence. The adapter is reusable for any future Calcite operator whose
PPL lowering goes through `WildcardUtils` (e.g. potential future patterns
in `like`-with-escape, custom regex lowerings).

Signed-off-by: Jialiang Liang <jiallian@amazon.com>
mch2 pushed a commit that referenced this pull request May 7, 2026
…/regexp_replace() functions (#21527)

Onboards the PPL `replace` command and the `replace()` / `regexp_replace()`
eval functions to the analytics-engine route by mapping their two Calcite
lowering targets — `SqlStdOperatorTable.REPLACE` and
`SqlLibraryOperators.REGEXP_REPLACE_3` — through Substrait to DataFusion's
native `replace` and `regexp_replace` UDFs.

Same templated shape as the `fillnull` POC (#21472):

  ScalarFunction enum constant
    + STANDARD_PROJECT_OPS membership
    + opensearch_scalar_functions.yaml extension entry
    + ADDITIONAL_SCALAR_SIGS Calcite-op→Substrait-name bridge
    = onboarded to the analytics route.

Two scalar functions added: REPLACE (literal substring replace) and
REGEXP_REPLACE (regex replace). Both project-side only; the comparison
result of a replaced field is filtered via the existing EQUALS capability,
so no STANDARD_FILTER_OPS additions are needed.

PPL's wildcard `replace` form lowers via `WildcardUtils.convertWildcardPatternToRegex()`
to a Java-`Pattern`-compatible regex. Two flavors of Java syntax need
translation before substrait serialization, because DataFusion uses Rust's
`regex` crate which has different parsing rules:

  * `\Q…\E` quoted-literal blocks — Rust rejects `\Q` as an unrecognized
    escape sequence. The adapter expands each block to per-character
    escaped literals (semantics-preserving).
  * `$N` numeric backreferences in the replacement — Rust's replacement
    parser is identifier-greedy, so `$1_$2` is parsed as a reference to
    group named `1_` followed by `$2` (Java parses it as group 1 + literal
    underscore + group 2). The adapter wraps every numeric backreference
    in braces (`${N}`) for unambiguous Rust parsing.

Both transforms are in `RegexpReplaceAdapter` and registered against
`ScalarFunction.REGEXP_REPLACE` in `scalarFunctionAdapters()`. Calls
without `\Q` in the pattern AND without bare `$N` in the replacement pass
through unchanged.

  * `RegexpReplaceAdapterTests` — 19/19 (unquote: 9, brace: 7, dual-rewrite
    integration: 3).
  * `ReplaceCommandIT` (new self-contained QA IT, calcs dataset) — 10/10.
    Covers literal command (single + multi-pair = nested REPLACE), wildcard
    command (prefix + suffix), `replace()` and `regexp_replace()` in eval,
    full-row content checks, no-match passthrough, multi-field IN clause.
  * SQL plugin's `CalciteReplaceCommandIT` force-routed through the
    analytics-engine route via `-Dtests.analytics.{force_routing,parquet_indices}=true`
    — 21/21 in both the direct suite and the `CalciteNoPushdownIT` re-run.
    (Companion SQL plugin PR #5415 makes 4 column-order assertions and 1
    error-message assertion order-agnostic, mirroring the rename precedent
    from #5413.)

Unlike `fillnull`/`regex` where the bridge was a single one-line capability
addition, `replace`'s wildcard form exposes Java↔Rust regex syntax
divergence. The adapter is reusable for any future Calcite operator whose
PPL lowering goes through `WildcardUtils` (e.g. potential future patterns
in `like`-with-escape, custom regex lowerings).

Signed-off-by: Jialiang Liang <jiallian@amazon.com>
imRishN pushed a commit to imRishN/OpenSearch that referenced this pull request May 8, 2026
…/regexp_replace() functions (opensearch-project#21527)

Onboards the PPL `replace` command and the `replace()` / `regexp_replace()`
eval functions to the analytics-engine route by mapping their two Calcite
lowering targets — `SqlStdOperatorTable.REPLACE` and
`SqlLibraryOperators.REGEXP_REPLACE_3` — through Substrait to DataFusion's
native `replace` and `regexp_replace` UDFs.

Same templated shape as the `fillnull` POC (opensearch-project#21472):

  ScalarFunction enum constant
    + STANDARD_PROJECT_OPS membership
    + opensearch_scalar_functions.yaml extension entry
    + ADDITIONAL_SCALAR_SIGS Calcite-op→Substrait-name bridge
    = onboarded to the analytics route.

Two scalar functions added: REPLACE (literal substring replace) and
REGEXP_REPLACE (regex replace). Both project-side only; the comparison
result of a replaced field is filtered via the existing EQUALS capability,
so no STANDARD_FILTER_OPS additions are needed.

PPL's wildcard `replace` form lowers via `WildcardUtils.convertWildcardPatternToRegex()`
to a Java-`Pattern`-compatible regex. Two flavors of Java syntax need
translation before substrait serialization, because DataFusion uses Rust's
`regex` crate which has different parsing rules:

  * `\Q…\E` quoted-literal blocks — Rust rejects `\Q` as an unrecognized
    escape sequence. The adapter expands each block to per-character
    escaped literals (semantics-preserving).
  * `$N` numeric backreferences in the replacement — Rust's replacement
    parser is identifier-greedy, so `$1_$2` is parsed as a reference to
    group named `1_` followed by `$2` (Java parses it as group 1 + literal
    underscore + group 2). The adapter wraps every numeric backreference
    in braces (`${N}`) for unambiguous Rust parsing.

Both transforms are in `RegexpReplaceAdapter` and registered against
`ScalarFunction.REGEXP_REPLACE` in `scalarFunctionAdapters()`. Calls
without `\Q` in the pattern AND without bare `$N` in the replacement pass
through unchanged.

  * `RegexpReplaceAdapterTests` — 19/19 (unquote: 9, brace: 7, dual-rewrite
    integration: 3).
  * `ReplaceCommandIT` (new self-contained QA IT, calcs dataset) — 10/10.
    Covers literal command (single + multi-pair = nested REPLACE), wildcard
    command (prefix + suffix), `replace()` and `regexp_replace()` in eval,
    full-row content checks, no-match passthrough, multi-field IN clause.
  * SQL plugin's `CalciteReplaceCommandIT` force-routed through the
    analytics-engine route via `-Dtests.analytics.{force_routing,parquet_indices}=true`
    — 21/21 in both the direct suite and the `CalciteNoPushdownIT` re-run.
    (Companion SQL plugin PR opensearch-project#5415 makes 4 column-order assertions and 1
    error-message assertion order-agnostic, mirroring the rename precedent
    from opensearch-project#5413.)

Unlike `fillnull`/`regex` where the bridge was a single one-line capability
addition, `replace`'s wildcard form exposes Java↔Rust regex syntax
divergence. The adapter is reusable for any future Calcite operator whose
PPL lowering goes through `WildcardUtils` (e.g. potential future patterns
in `like`-with-escape, custom regex lowerings).

Signed-off-by: Jialiang Liang <jiallian@amazon.com>
Bukhtawar pushed a commit to Bukhtawar/OpenSearch that referenced this pull request May 10, 2026
…/regexp_replace() functions (opensearch-project#21527)

Onboards the PPL `replace` command and the `replace()` / `regexp_replace()`
eval functions to the analytics-engine route by mapping their two Calcite
lowering targets — `SqlStdOperatorTable.REPLACE` and
`SqlLibraryOperators.REGEXP_REPLACE_3` — through Substrait to DataFusion's
native `replace` and `regexp_replace` UDFs.

Same templated shape as the `fillnull` POC (opensearch-project#21472):

  ScalarFunction enum constant
    + STANDARD_PROJECT_OPS membership
    + opensearch_scalar_functions.yaml extension entry
    + ADDITIONAL_SCALAR_SIGS Calcite-op→Substrait-name bridge
    = onboarded to the analytics route.

Two scalar functions added: REPLACE (literal substring replace) and
REGEXP_REPLACE (regex replace). Both project-side only; the comparison
result of a replaced field is filtered via the existing EQUALS capability,
so no STANDARD_FILTER_OPS additions are needed.

PPL's wildcard `replace` form lowers via `WildcardUtils.convertWildcardPatternToRegex()`
to a Java-`Pattern`-compatible regex. Two flavors of Java syntax need
translation before substrait serialization, because DataFusion uses Rust's
`regex` crate which has different parsing rules:

  * `\Q…\E` quoted-literal blocks — Rust rejects `\Q` as an unrecognized
    escape sequence. The adapter expands each block to per-character
    escaped literals (semantics-preserving).
  * `$N` numeric backreferences in the replacement — Rust's replacement
    parser is identifier-greedy, so `$1_$2` is parsed as a reference to
    group named `1_` followed by `$2` (Java parses it as group 1 + literal
    underscore + group 2). The adapter wraps every numeric backreference
    in braces (`${N}`) for unambiguous Rust parsing.

Both transforms are in `RegexpReplaceAdapter` and registered against
`ScalarFunction.REGEXP_REPLACE` in `scalarFunctionAdapters()`. Calls
without `\Q` in the pattern AND without bare `$N` in the replacement pass
through unchanged.

  * `RegexpReplaceAdapterTests` — 19/19 (unquote: 9, brace: 7, dual-rewrite
    integration: 3).
  * `ReplaceCommandIT` (new self-contained QA IT, calcs dataset) — 10/10.
    Covers literal command (single + multi-pair = nested REPLACE), wildcard
    command (prefix + suffix), `replace()` and `regexp_replace()` in eval,
    full-row content checks, no-match passthrough, multi-field IN clause.
  * SQL plugin's `CalciteReplaceCommandIT` force-routed through the
    analytics-engine route via `-Dtests.analytics.{force_routing,parquet_indices}=true`
    — 21/21 in both the direct suite and the `CalciteNoPushdownIT` re-run.
    (Companion SQL plugin PR opensearch-project#5415 makes 4 column-order assertions and 1
    error-message assertion order-agnostic, mirroring the rename precedent
    from opensearch-project#5413.)

Unlike `fillnull`/`regex` where the bridge was a single one-line capability
addition, `replace`'s wildcard form exposes Java↔Rust regex syntax
divergence. The adapter is reusable for any future Calcite operator whose
PPL lowering goes through `WildcardUtils` (e.g. potential future patterns
in `like`-with-escape, custom regex lowerings).

Signed-off-by: Jialiang Liang <jiallian@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants