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

Default types replaced with pgtype wherever possible #789

Merged
merged 4 commits into from
Dec 11, 2023

Conversation

Amogh-Bharadwaj
Copy link
Contributor

Also set connection pool limit to 3 for our postgres client

@serprex
Copy link
Contributor

serprex commented Dec 11, 2023

What's the benefit of this? It seems like an unnecessary intermediary compared to using go's primitive types

jackc/pgtype#44 seems to give some perspective, but wanting to know experience on our end

@iskakaushik
Copy link
Contributor

@serprex the reason was null handling is handled better with these types. When scanning a null string, we encountered an NPE. So pre-emptively moving to this. It might make more sense to inspect each call to see if the extra type is needed, but given that most of these calls are to secondary to mirror perf, I am inclined to be ok with using these types.

@Amogh-Bharadwaj Amogh-Bharadwaj force-pushed the scan-types-and-connection-limit branch from 0cc39cc to b8f4d47 Compare December 11, 2023 15:23
Copy link
Contributor

@serprex serprex left a comment

Choose a reason for hiding this comment

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

should explain more in PR description so information is in git history

@iskakaushik iskakaushik enabled auto-merge (squash) December 11, 2023 15:35
@iskakaushik iskakaushik merged commit 765cdba into main Dec 11, 2023
12 checks passed
@serprex serprex deleted the scan-types-and-connection-limit branch July 19, 2024 15:20
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.

3 participants