Skip to content

fix(generator): emit ROWS keyword after OFFSET for Oracle (CR-011)#17

Merged
yigalrozenberg merged 1 commit into
masterfrom
fix/cr-011-oracle-offset-rows-keyword
May 28, 2026
Merged

fix(generator): emit ROWS keyword after OFFSET for Oracle (CR-011)#17
yigalrozenberg merged 1 commit into
masterfrom
fix/cr-011-oracle-offset-rows-keyword

Conversation

@yigalrozenberg

Copy link
Copy Markdown
Collaborator

Summary

Fixes CR-011 / PSQ-2422: Oracle generator omitted the mandatory ROWS keyword after OFFSET n, causing ORA-02000: missing ROWS keyword on all OFFSET-FETCH pagination queries routed through the gateway.

Root cause

The CR-010 fix (v0.9.36) added ROWS emission for Tsql | Fabric but did not include Oracle, which requires the same SQL:2008 syntax: OFFSET n ROWS FETCH FIRST m ROWS ONLY.

Fix

One-line change in src/generator/sql_generator.rs: add | Some(Dialect::Oracle) to the match pattern.

Tests added (tests/test_dialects.rs)

  • test_oracle_offset_fetch_roundtrip — identity roundtrip
  • test_oracle_offset_only_rows_keyword — OFFSET without FETCH
  • test_pg_to_oracle_limit_offset — PG LIMIT/OFFSET → Oracle OFFSET/FETCH FIRST
  • test_pg_to_oracle_limit_offset_no_order_by — confirms Oracle doesn't need ORDER BY guard

Full suite green (1058+ tests, 0 failures). No regressions.

Refs: PSQ-2422

…-011)

Oracle's SQL:2008 pagination syntax requires 'OFFSET n ROWS' — without
the ROWS keyword, Oracle raises ORA-02000. The generator's OFFSET emission
was gated behind Tsql|Fabric only (from the CR-010 fix). Oracle requires
the same keyword.

Fix: add Dialect::Oracle to the match pattern that emits ROWS after OFFSET.

Tests (tests/test_dialects.rs):
- test_oracle_offset_fetch_roundtrip (identity roundtrip)
- test_oracle_offset_only_rows_keyword (OFFSET without FETCH)
- test_pg_to_oracle_limit_offset (PG LIMIT/OFFSET → Oracle)
- test_pg_to_oracle_limit_offset_no_order_by (no ORDER BY case)

Refs: PSQ-2422
@yigalrozenberg yigalrozenberg merged commit 8b84950 into master May 28, 2026
4 checks 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.

1 participant