Skip to content

fix: rewrite limit / offset - #16

Merged
sweb merged 2 commits into
mainfrom
fix/rewrite-limit-offset
Aug 11, 2026
Merged

fix: rewrite limit / offset#16
sweb merged 2 commits into
mainfrom
fix/rewrite-limit-offset

Conversation

@sweb

@sweb sweb commented Jul 20, 2026

Copy link
Copy Markdown
Member

No description provided.

@lfrancke lfrancke moved this to Development: Waiting for Review in Stackable Engineering Jul 24, 2026

@lfrancke lfrancke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

minor stuff

Comment thread src/rewrite/mod.rs
/// - PostgreSQL type names are normalized to Trino equivalents
/// - `ILIKE` becomes `lower(x) LIKE lower(pattern)`
/// - PostgreSQL function names are mapped to Trino equivalents
/// - `LIMIT n OFFSET m` is reordered into Trino order

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We also list the rewrites we do in the README and AGENTs file. We should either remove that stuff or update it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

addressed in 056053d

Comment thread src/rewrite/limit_offset.rs Outdated
query.fetch = Some(Fetch {
with_ties: false,
percent: false,
quantity: Some(limit),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you check whether/how Trino handles the edge case "0" here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

you are right, this did cause an error and thank you for noticing - addressed in 056053d

@lfrancke lfrancke moved this from Development: Waiting for Review to Development: In Review in Stackable Engineering Jul 30, 2026
@sweb
sweb merged commit 7376c01 into main Aug 11, 2026
5 checks passed
@sweb
sweb deleted the fix/rewrite-limit-offset branch August 11, 2026 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Development: In Review

Development

Successfully merging this pull request may close these issues.

2 participants