fix(oracle): strip trailing semicolon before subquery-wrapping#2423
fix(oracle): strip trailing semicolon before subquery-wrapping#2423Bartok9 wants to merge 1 commit into
Conversation
Both OracleConnector.query (with a limit) and dry_run wrap user SQL as
SELECT * FROM ({sql}) t WHERE ROWNUM <= N. Oracle rejects a trailing
semicolon inside a subquery (ORA-00911: invalid character), so a query
ending in ; fails even though it runs fine standalone. Strip only the
terminating semicolon/whitespace run so semicolons inside string literals
are preserved. Mirrors the postgres/canner/trino/clickhouse connectors.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds a ChangesOracle Trailing Semicolon Stripping
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
OracleConnector.query(when a limit is given) andOracleConnector.dry_runwrap user SQL asSELECT * FROM ({sql}) t WHERE ROWNUM <= N. When the user SQL ends in;, the subquery is invalid and Oracle raisesORA-00911: invalid character.Motivation
The sibling connectors already strip a trailing semicolon before subquery-wrapping: postgres, canner, trino, clickhouse all have a
_strip_trailing_semicolonhelper. Oracle wrapped raw SQL in both code paths.The fix adds
_strip_trailing_semicolon()(same[;\s]+\Zregex as the postgres connector) and applies it in bothquery()anddry_run(). Only the terminating run of semicolons/whitespace is stripped, so semicolons inside string literals (SELECT 'a;b') are preserved.Verification
uv run --no-sync pytest tests/unit/test_oracle_semicolon.py -q— 4 passed (with theoracleextra installed).Real behavior proof
Regression test
core/wren/tests/unit/test_oracle_semicolon.pyasserts the executed SQL and FAILS without the fix. Itpytest.importorskip("oracledb")so it skips cleanly when the oracle extra isn't installed.Without the fix (helper absent):
With the fix:
The query test asserts the executed SQL is exactly
SELECT * FROM (SELECT 1) t WHERE ROWNUM <= 5(no;)); the dry_run test asserts... WHERE ROWNUM <= 0.Scope / risk
core/wren/src/wren/connector/oracle.py(Apache-2.0 path) + a new unit test.query()path (which doesn't wrap) is unchanged; string-literal semicolons preserved (covered by a helper test).Summary by CodeRabbit