Skip to content

Conversation

@eaglesemanation
Copy link

With this addition, it should be possible to implement codegen on pgx/v5 side to resolve #2760
. Not sure for other drivers.

I intend to implement codegen as part of this PR as well, just want to get visibility as soon as possible.

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Mar 29, 2025
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Apr 4, 2025
@eaglesemanation
Copy link
Author

Intentionally left endtoend test failing, until I verify that generated code does work as intended

@eaglesemanation
Copy link
Author

Tested this, breaks if a column is an array of composite type. Otherwise works fine, except that pgx.Tx does not implement LoadTypes(...), will look into it soon

@PiotrBaczkowski96
Copy link

any update?

will it get merged it?

@eaglesemanation
Copy link
Author

Doesn't look like anyone had a chance to review this PR, and I haven't reached out to anyone to speed this up. I'm surprised that it still doesn't have any conflicts, so I don't mind making small adjustments to get this merged if needed.

@andrewbenton
Copy link

I've been using a slightly modified version of this for a couple of months now. Overall, it's a really great improvement that I hope to see merged upstream soon.

I have noticed a few things that would help improve it:

  1. toposort doesn't produce consistent ordering of composite types in generated files over multiple invocations of sqlc generate. I'm fairly certain that's due to the nature of Go's maps having a randomized key order over different process invocations. I think this could be corrected by making sortedCompositeTypes and toposort visit keys in alphabetical order.
  2. Arrays inside of composite types don't seem to be supported. They'll be emitted instead as the non-array type of their value, e.g. bool array becomes bool, text array becomes text, etc
  3. This could be my own misunderstanding or preference speaking, but I found that not changing the original DB interface was easier to work with. Instead, I introduced a new type TypeRegistrar interface that's able to do the type registration and emitted a func RegisterTypes(ctx context.Context, tr TypeRegistrar) error that can be used when setting up a connection. This allows types to be registered once right after the connection is established e.g. via pgxpool's AfterConnect hook.

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

Labels

size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support composite types

3 participants