Skip to content

change the Oracle select to use OFFSET...FETCH#231

Open
zhangyongding wants to merge 2 commits intohuandu:masterfrom
zhangyongding:chore-oralce-select-offset-fetch
Open

change the Oracle select to use OFFSET...FETCH#231
zhangyongding wants to merge 2 commits intohuandu:masterfrom
zhangyongding:chore-oralce-select-offset-fetch

Conversation

@zhangyongding
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Jan 22, 2026

Coverage Status

coverage: 97.124% (+0.1%) from 97.01%
when pulling 9bae871 on zhangyongding:chore-oralce-select-offset-fetch
into 03c69da on huandu:master.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR modernizes Oracle pagination by replacing the legacy ROWNUM-based approach with the standard OFFSET...FETCH syntax (introduced in Oracle 12c). This brings Oracle's SQL generation in line with modern SQL standards and simplifies the codebase.

Changes:

  • Removed complex ROWNUM-based pagination logic from select.go
  • Implemented OFFSET...FETCH syntax for Oracle in select.go (matching the already-updated union.go)
  • Updated test expectations in select_test.go to reflect the new Oracle query format

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
select.go Replaced ROWNUM-based pagination with OFFSET...FETCH syntax; removed unused strings import and ~50 lines of complex nested query logic
select_test.go Updated Oracle test expectations to show the new OFFSET...FETCH query format instead of ROWNUM queries

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@zhangyongding
Copy link
Contributor Author

Can it be merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants