r2dbc-postgresql icon indicating copy to clipboard operation
r2dbc-postgresql copied to clipboard

PostgresTypes returns types that aren't visible in the current schema

Open osi opened this issue 4 years ago • 9 comments
trafficstars

  • Driver: 0.8.6

PostgresTypes does a LEFT JOIN against the namespaces, resulting in multiple types being returned if the type is defined in multiple namespaces

https://github.com/pgjdbc/r2dbc-postgresql/blob/ff5c93c03090b18217edac3c93cedda40768cc72/src/main/java/io/r2dbc/postgresql/codec/PostgresTypes.java#L38-L39

I have a type that is defined in multiple schemas (the schema is a "version", and application code is only using a specific version). This is controlled by the active schema, and current_schemas will only return a single one. If I have a type defined in 3 schemas, I will receive three results from PostgresTypes as it does a LEFT JOIN against the visible namespaces rather than an INNER JOIN.

If the LEFT JOIN behavior is desired, adding the namespace to the returned object would allow filtering in application code.

osi avatar Nov 24 '20 17:11 osi

We should be doing what PGJDBC does. /cc @davecramer

mp911de avatar Nov 24 '20 19:11 mp911de

PGJDBC only keeps the first match for a name,

https://github.com/pgjdbc/pgjdbc/blob/51f18bf7755b97a4ab3e560431a3578d119a2bdf/pgjdbc/src/main/java/org/postgresql/jdbc/TypeInfoCache.java#L202-L206

https://github.com/pgjdbc/pgjdbc/blob/master/pgjdbc/src/main/java/org/postgresql/jdbc/TypeInfoCache.java#L265-L267

.. I was naively assuming (looking at the API surface) that I'd only get a single result for a name out of lookupTypes. I can think of the following ways to address this:

  • Solve via documentation that duplicates will be returned, and that duplicates will be ordered based on the order of the schema in the search path
  • The above, but also providing the containing schema name on io.r2dbc.postgresql.codec.PostgresTypes.PostgresType, so users would know the containing schema to pick their match
  • Have lookupTypes only return the first match for a given name (similar to how lookupType will only return one match)

osi avatar Nov 24 '20 20:11 osi

Also, if using the provided EnumCodec with a registration priority of FIRST will result in an enum that's present in multiple schemas to have the one from the end of the search path as the first choice (which seems undesirable)

osi avatar Nov 24 '20 20:11 osi

@osi Since it is actually possible to have a valid type with the same name in two different active schemas it seems to me that we need to return a schema qualified type.

Thoughts?

davecramer avatar Dec 08 '20 16:12 davecramer

@davecramer that'd be the least ambiguous option when there are multiple active schemas. (that's actually how I discovered this problem, when the server gave me an error with the schema-qualified name that it was seeing vs expected)

osi avatar Dec 08 '20 17:12 osi

@osi OK, I'll take a run at implementing that, unless you have some cycles ?

davecramer avatar Dec 08 '20 17:12 davecramer

I'l have cycles by end-of-year, if that timeframe works.

osi avatar Dec 08 '20 17:12 osi

if it's still around by then, have at it.

davecramer avatar Dec 08 '20 18:12 davecramer

I went to work on this and realized it isn't a problem with enums. My initial observation was with a custom type, and I was using CodecRegistry#addFirst instead of CodecRegistry#addLast, so the types that appeared later in the schema search path were given greater priority in the registry, leading to their use.

If anything I'd say we could do this,

Solve via documentation that duplicates will be returned, and that duplicates will be ordered based on the order of the schema in the search path

.. using addFirst is likely not what is desired when adding additional type mappings. In my code, i was using addFirst as I have a couple of codec's that I want to run before some of the built-ins (to create custom value types in my domain).

osi avatar Dec 22 '20 21:12 osi