Skip to content

Fix negative from index handling in INSTR - #820

Draft
lkxdsb wants to merge 1 commit into
apache:masterfrom
lkxdsb:codex/geaflow-issue-87
Draft

Fix negative from index handling in INSTR#820
lkxdsb wants to merge 1 commit into
apache:masterfrom
lkxdsb:codex/geaflow-issue-87

Conversation

@lkxdsb

@lkxdsb lkxdsb commented Jul 29, 2026

Copy link
Copy Markdown

What changes were proposed in this pull request?

This PR fixes INSTR when the from argument is negative.

  • interpret a negative from as a position counted backward from the end of the string
  • search backward from that position and return the nth matching occurrence
  • keep return values 1-based and return 0 when no occurrence can be found
  • apply identical semantics to the String and BinaryString overloads
  • preserve the existing behavior for positive from, from = 0, invalid nth, and null arguments
  • add both focused unit tests and a local GeaFlow runtime integration test

Why are the changes needed?

The previous implementation always converted from to a zero-based forward-search index:

int fromIndex = from.intValue() - 1;

A negative value therefore entered the invalid-index branch and returned null, instead of locating a starting position from the end of the string and searching backward. This produced incorrect results for the negative-index behavior described in #87.

Fixes #87

Behavior after this change

Expression Result Explanation
INSTR('abcabc', 'a', -1, 1) 4 last a when searching backward from the end
INSTR('abcabc', 'a', -1, 2) 1 second a encountered while searching backward
INSTR('abcabc', 'c', -3, 1) 3 search starts at the position selected by -3
INSTR('aaaa', 'aa', -1, 2) 2 overlapping occurrences remain searchable
INSTR('abcabc', 'a', -7, 1) 0 starting position is outside the string

Implementation details

  • The String overload uses String.lastIndexOf for the negative search path.
  • BinaryString does not provide lastIndexOf, so the implementation retains the last forward match not beyond the requested starting position.
  • Advancing the BinaryString scan by one character preserves overlapping matches.
  • Positive-index handling remains on the existing forward-search path to minimize behavior changes outside this issue.

How was this PR tested?

Unit test

JAVA_HOME=$(/usr/libexec/java_home -v 11) \
mvn -pl geaflow/geaflow-dsl/geaflow-dsl-plan \
-Dtest=UDFInstrTest test

Result:

Tests run: 2, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

The unit test covers negative positions, multiple backward matches, overlapping matches, out-of-range positions, multibyte strings, and both input types.

Local runtime integration test

JAVA_HOME=$(/usr/libexec/java_home -v 11) \
mvn -pl geaflow/geaflow-dsl/geaflow-dsl-runtime \
-Dtest=InstrRuntimeTest test

Result:

Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

This test starts the local GeaFlow environment, submits the SQL through GQLPipeLine, executes it with the file connector, and compares the generated Sink output with the expected result:

4,1,3,2,0

Additional checks

  • Checkstyle: 0 violations
  • Apache RAT: 0 unapproved licenses
  • git diff --check: passed

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.

Fix return value incorrect when fromIndex parameter is negative

1 participant