Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Continue sqlc migration #385

Merged
merged 1 commit into from
Apr 26, 2024
Merged

Continue sqlc migration #385

merged 1 commit into from
Apr 26, 2024

Conversation

neekolas
Copy link
Collaborator

@neekolas neekolas commented Apr 26, 2024

tl;dr

  • Migrates the rest of the queries to sqlc, including some helper queries only used in tests
  • Deletes all the Bun models
  • With Bun gone, I turned off the Simple Execution Mode, which allows pgx to use prepared statement caching (probably a pretty minor performance improvement, but can't hurt)

What's next

No urgency at all on these, but at some point I'd like to get rid of the rest of Bun in the repo and remove the dependency. That would involve:

  • Replacing bun.Migrate with another migration tool and porting over our migrations
  • Replacing the authz database with sqlc queries (should be super easy, there's maybe 2 queries)

Copy link
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @neekolas and the rest of your teammates on Graphite Graphite

SELECT
*
FROM
group_messages
WHERE
group_id = @group_id
ORDER BY
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm using this fancy case statement for sorting for now. It seems to work fine, although I'm a little worried about perf. Sometimes using a case statement will prevent the query planner from using indexes. Will try and test

Copy link

Choose a reason for hiding this comment

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

Curious about how you would do this. Would pgbench and tools that run thorough SQL transcripts be considered a practical approach?

@neekolas neekolas force-pushed the 04-25-continue_sqlc_migration branch from 1163957 to c2f0240 Compare April 26, 2024 15:43
@neekolas neekolas marked this pull request as ready for review April 26, 2024 18:34
@neekolas neekolas requested review from insipx and 37ng April 26, 2024 18:52
SELECT
*
FROM
group_messages
WHERE
group_id = @group_id
ORDER BY
Copy link

Choose a reason for hiding this comment

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

Curious about how you would do this. Would pgbench and tools that run thorough SQL transcripts be considered a practical approach?

@neekolas
Copy link
Collaborator Author

@37ng pgbench is great. I was thinking just the simplest approach of running the query with and without the CASE statement through EXPLAIN ANALYZE (which we can do in retool) and seeing if the query plan is actually different.

@37ng
Copy link

37ng commented Apr 26, 2024

through EXPLAIN ANALYZE (which we can do in retool) and seeing if the query plan is actually different.

@neekolas That's pretty cool!

@neekolas
Copy link
Collaborator Author

@37ng did the test in Retool and the query plan looks identical

@neekolas neekolas merged commit ff22ba5 into main Apr 26, 2024
7 checks passed
@neekolas neekolas deleted the 04-25-continue_sqlc_migration branch April 26, 2024 22:43
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