Skip to content

Cache queries regardless of routing comments - #1752

Merged
sgrif merged 2 commits into
mainfrom
sg-cache-commented
Oct 8, 2026
Merged

sgrif merged 2 commits into
mainfrom
sg-cache-commented

Conversation

@sgrif

@sgrif sgrif commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Routing comments do not affect the rewriter in any way, and routing happens after rewriting occurs. There's no reason we need to avoid caching these

Routing comments do not affect the rewriter in any way, and routing
happens after rewriting occurs. There's no reason we need to avoid
caching these
@sgrif
sgrif requested a review from levkk October 7, 2026 21:41
@codecov

codecov Bot commented Oct 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@levkk

levkk commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

There's no reason we need to avoid caching these

I remember disabling caching for rewritten queries, but I can't remember why. Maybe the commit history around that code could help.

@sgrif

sgrif commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Yeah I looked through the history but it was mixed in with massive features and provided no context around why this was the case. The struct that went into the cache did previously have the comment information on it, but that was overwritten every time it got pulled out of the cache anyway. AFAIK there hasn't ever been a time where routing information affected rewriting, and the code's current structure makes this even more apparent because the function that handles it couldn't access that information even if it wanted to

@levkk

levkk commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

I'm remembering I think. It was because the routing comment would have a different execution plan than the non-commented one, e..g, /* pgdog_shard: 0 */ would invalidate the rewrite plan. In the future, if we received a query that was rewritten and cached it anyway, the same query with a comment would hit the cache and execute the rewritten plan, not the direct-to-shard plan.

I don't think this is relevant anymore since we have two different plans now: one for direct and for cross, since we moved the aggregate/ordering/etc to post-route. #1545

@sgrif
sgrif added this pull request to the merge queue Oct 8, 2026
Merged via the queue into main with commit 4dc0614 Oct 8, 2026
46 checks passed
@sgrif
sgrif deleted the sg-cache-commented branch October 8, 2026 16:18
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.

2 participants