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

fix: remove to_plural from postgres connector runtime #1718

Merged
merged 1 commit into from
May 15, 2024

Conversation

obmarg
Copy link
Member

@obmarg obmarg commented May 15, 2024

I was digging into the binary sizes of the gateway & executor last week. One of the things I noticed was that the regex crate (a known space waster) was being pulled into the executor, so I dug into that. Turns out this was from a single innocent looking call to Inflector::to_plural in the postgres connector.

So, this works the connector to do that call at deploy time in the postgres-parser instead of at runtime, which should reduce the executor payload by a fair bit.

I've approached this by just storing the singular & plural relation field names against every table and storing that. I'm not 100% happy with this implementation: these are technically only required for tables with relations, and even then a given relation might only need one of the forms. But I'm not familiar enough with the code to figure out the best place to do exactly what we need. Particularly since tables names can change later in the process if their name clashes, so we'd then need to propagate that into our memoised relation names. If anyone knows of a better way to do this (or wants to revisit) feel free.

Part of GB-6697

@obmarg obmarg requested a review from a team as a code owner May 15, 2024 09:10
@obmarg obmarg requested a review from yoav-lavi May 15, 2024 09:10
Copy link
Contributor

@tomhoule tomhoule left a comment

Choose a reason for hiding this comment

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

💯

Copy link
Collaborator

@yoav-lavi yoav-lavi left a comment

Choose a reason for hiding this comment

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

Nice!

@obmarg obmarg merged commit ac1921a into main May 15, 2024
21 checks passed
@obmarg obmarg deleted the obmarg-push-pusuqkmkyuxt branch May 15, 2024 12:44
Copy link

linear bot commented May 15, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants