Switch paginated queries to use psql tuple comparison to improve planner performance#8066
Switch paginated queries to use psql tuple comparison to improve planner performance#8066jdolle wants to merge 2 commits into
Conversation
|
🐋 This PR was built and pushed to the following Docker images: Targets: Platforms: Image Tag: |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The refactor to PostgreSQL tuple comparisons for pagination contains several critical bugs. The table alias 'c' is used in multiple queries where it is not defined in the 'FROM' clause, leading to runtime failures. Furthermore, the 'organization_members' query incorrectly references 'id' instead of 'user_id' for the cursor comparison.
| ? psql` | ||
| AND ( | ||
| ( | ||
| c."created_at" = ${cursor.createdAt} |
There was a problem hiding this comment.
This was a bug before. I guess we werent ever using this?
There was a problem hiding this comment.
What is the bug that was here?
There was a problem hiding this comment.
It's referencing c. but nothing is being aliased to c.
There was a problem hiding this comment.
AFAIK these two approaches are functional equivalent. I personally use the AND OR over the tuple approach as it is more explicit to me. But I can be convinced to use these instead if it provides some benefit.
I will still reject this unless I am convinced to either
- why this improves performance
- why this is more readable and understandable (without having to open the documentation)
|
I'm fine closing this if we're not comfortable with the syntax. Personally I find it much easier to read since it's very compact. But if we prefer the verbose form, it's functionally equivalent. |
2 participants
Background
Performance improvements
Description
This query is the same as the OR but is easier on the planner