Skip to content

Conversation

dungeon-master-666
Copy link
Contributor

Currently custom composite types from postgres are saved as strings in CH which makes it inconvenient to access/filter/sort/etc them. This PR adds functionality converting composite types to Tuple.

  1. Nested composite types and arrays of composites are supported
  2. Clickhouse doesn't have Nullable(Tuple(...)) so instead every field of tuple is created Nullable (if PEERDB_NULLABLE is enabled)
  3. Apparently this feature should be wrapped in setting to make existing mirrors won't break.
  4. I tested only the direct scenario, PG -> CH mirror. PG -> PG and others were not tested

@serprex serprex marked this pull request as ready for review July 10, 2025 23:46
@serprex serprex marked this pull request as draft July 10, 2025 23:46
@serprex
Copy link
Member

serprex commented Jul 10, 2025

#3066 introduced versioning as a simpler alternative to settings for maintaining backwards compatibility

@serprex serprex requested a review from heavycrystal July 11, 2025 14:07
@serprex
Copy link
Member

serprex commented Aug 19, 2025

wanted to get to bringing this to a point we can merge, but been falling behind this past month

@dungeon-master-666
Copy link
Contributor Author

wanted to get to bringing this to a point we can merge, but been falling behind this past month

I'm doing a bit of refactoring right now and fixing several issues. Will be ready for review in a day or 2.

@dungeon-master-666
Copy link
Contributor Author

Hi @serprex @heavycrystal, it's ready for reviewing. I've added a new version CompositeTypeAsTuple for backward compatibility and fixed several issues with arrays of composites and nested tuples.

@dungeon-master-666 dungeon-master-666 marked this pull request as ready for review August 22, 2025 15:03
@serprex serprex mentioned this pull request Aug 22, 2025
compositeTypeNames = append(compositeTypeNames, typeData.Name)
}
}
types, err := c.conn.LoadTypes(ctx, compositeTypeNames)
Copy link
Member

Choose a reason for hiding this comment

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

this is failing in CI for #3384

failed to fetch custom type mapping: failed to load composite types: Unknown field for composite type "geometry_dump":  field "geom" (OID 18053) is not already registered.

seems like there's a mess with having to make sure dependencies are registered before loading. Looking at LoadTypes this can probably be cleaned up to query catalog ourselves for composite types specifically & figure out how to register types in correct order

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weird, LoadTypes already has some logic for registering children types before parents: https://github.com/jackc/pgx/blob/master/derived_types.go#L82-L84

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