Skip to content

[CALCITE-7761] Preserve OFFSET and FETCH when rewriting outer ORDER BY - #5238

Open
tkalkirill wants to merge 3 commits into
apache:mainfrom
tkalkirill:calcite-7761
Open

[CALCITE-7761] Preserve OFFSET and FETCH when rewriting outer ORDER BY#5238
tkalkirill wants to merge 3 commits into
apache:mainfrom
tkalkirill:calcite-7761

Conversation

@tkalkirill

@tkalkirill tkalkirill commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Jira Link

CALCITE-7761

Changes Proposed

Preserves OFFSET and FETCH added during query rewriting when an outer ORDER BY is present.
For example, the limit in this query is no longer lost during validation:

SELECT * FROM person WHERE ROWNUM <= 5 ORDER BY id;

@tkalkirill tkalkirill changed the title Calcite 7761 [CALCITE-7761] Preserve OFFSET and FETCH when rewriting outer ORDER BY Sep 2, 2026
@julianhyde

Copy link
Copy Markdown
Contributor

Seriously, is this the test case you want to merge to main?

@tkalkirill

Copy link
Copy Markdown
Contributor Author

Seriously, is this the test case you want to merge to main?

You’re right, the previous test was too synthetic. I’ve replaced it with a SQL-level regression test based on the reported ROWNUM case. It simulates the ROWNUM-to-FETCH rewrite and verifies that the outer ORDER BY preserves the rewritten FETCH.

@sonarqubecloud

sonarqubecloud Bot commented Sep 2, 2026

Copy link
Copy Markdown

if (select.getOrderList() == null) {
// Don't clobber existing ORDER BY, OFFSET, or FETCH. ORDER BY may be
// needed for an order-sensitive function like RANK; OFFSET and FETCH
// establish a separate pagination level.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know what "pagination" means. this term does not appear in the codebase.

The fix looks safe, though.

}
}

/** Validator that simulates rewriting a ROWNUM predicate to FETCH. */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think @julianhyde asked for a quidem test: a SQL program in a .iq file.
If that exercises this case, it's better than a test such as this one.

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.

3 participants