Skip to content

Fix openGauss LIMIT offset, row-count parameter marker order#38647

Open
daguimu wants to merge 1 commit into
apache:masterfrom
daguimu:fix/opengauss-limit-offset-param-order-34961
Open

Fix openGauss LIMIT offset, row-count parameter marker order#38647
daguimu wants to merge 1 commit into
apache:masterfrom
daguimu:fix/opengauss-limit-offset-param-order-34961

Conversation

@daguimu
Copy link
Copy Markdown

@daguimu daguimu commented Apr 24, 2026

Problem

On ShardingSphere-Proxy against openGauss, a prepared statement that uses the MySQL-style LIMIT offset, count shortcut with parameter markers returns rows in the wrong window. For example, SELECT * FROM employees ORDER BY id LIMIT ?, ? bound with setInt(1, 2); setInt(2, 1) is expected to skip 2 rows and return 1 row, but ShardingSphere treats the first ? as row-count and the second ? as offset, so it returns 2 rows starting from offset 1.

Root Cause

OpenGaussStatementVisitor.createLimitSegmentWhenRowCountOrOffsetAbsent builds the LimitSegment for the LIMIT selectOffsetValue COMMA_ selectLimitValue grammar branch by visiting selectLimitValue before selectOffsetValue. Parameter marker indices are assigned from the visitor counter in visit order (see OpenGaussStatementVisitor.visitParameterMarker and the parameterMarkerSegments collection), not from lexical position. Visiting the row-count child first therefore gives the second ? index 0 and the first ? index 1, which is the reverse of the user-facing parameter order.

Fix

Visit selectOffsetValue first, then selectLimitValue, in createLimitSegmentWhenRowCountOrOffsetAbsent. Parameter marker indices now line up with lexical order, matching how DorisStatementVisitor.visitLimitClause already handles the identical MySQL-style shortcut.

Tests Added

Added OpenGaussStatementVisitorTest under parser/sql/engine/dialect/opengauss/src/test:

Change point Test
Swap visit order so LIMIT ?, ? offset gets index 0 and row-count gets index 1 assertVisitLimitWithOffsetAndRowCountParameterMarkers - asserts offset.parameterIndex == 0, rowCount.parameterIndex == 1 (fails without the fix, passes with it)
Regression: literal LIMIT n, m still parses correctly assertVisitLimitWithLiteralOffsetAndRowCount - asserts offset value 1 and row-count value 2
Regression: single-marker LIMIT ? unchanged assertVisitLimitWithOnlyRowCountParameterMarker - asserts row-count parameter index 0 and no offset
Regression: separate OFFSET ? LIMIT ? clauses unchanged assertVisitOffsetAndLimitWithParameterMarkers - asserts offset parameter index 0 and row-count parameter index 1

Impact

  • Touches only the MySQL-style LIMIT offset, count branch in the openGauss visitor.
  • Literal-valued LIMIT n, m, plain LIMIT ?, LIMIT ? OFFSET ?, and FETCH paths are unchanged.
  • PostgreSQL parser is not affected (its grammar does not accept the LIMIT offset, count shortcut).

Fixes #34961

openGauss supports MySQL-style `LIMIT offset, count` shortcut. The
visitor was building the LimitSegment by visiting the row-count child
before the offset child, so parameter marker indices were assigned in
reverse lexical order.

With `LIMIT ?, ?`, the first `?` would be bound to row-count and the
second `?` to offset, making user parameters arrive swapped at runtime.

Visit offset first, then row-count, to match lexical order.

Fixes apache#34961
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.

In the opengauss database, when using limit ?, ?, the parameter parsing order is wrong

1 participant