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

Composite types can not be safely encode or decoded #353

Open
fishcakez opened this issue Nov 25, 2017 · 1 comment
Open

Composite types can not be safely encode or decoded #353

fishcakez opened this issue Nov 25, 2017 · 1 comment

Comments

@fishcakez
Copy link
Member

fishcakez commented Nov 25, 2017

If a composite type, such as a table row, changes its types then we do not update the types and oids. For encoding this means we send the wrong types to the database and error. For decoding it means we crash because oids don't match. Similarly for decoding anonymous composites it possible that it includes a type that we have not bootstrapped because the preparation of the query won't signal the missing type. In this situation we crash too. Catching any of these situations in a user's test is very unlikely.

I am unsure if this has been reported(elixir-ecto/ecto#1271 (comment)) . It would be very difficult to diagnose because of the complicated way postgrex handles types. Also I am unsure how much composites are used.

For encoding we can resolve this by looking up the composite types at parameter describe time - we don't support anonymous composites. If we require a redescribe (executing on new connection with known parameters types but unknown result types) and types changed then we will get a postgresql error because we don't lookup parameters on redescribe (1eb868). Doing a query on every composite prepare is undesirable but should work. We would be logging a warning every time this occurred. For performance people may choose not to use this but existing statements would still work.

For decoding we can partially resolve the internal types changing by treating all composites types as anonymous. This means we delay resolving the oids until decoding, or perhaps only if oids have changed fallback to the anonymous approach. However this means we need to be able to decode anonymous types.

Alternatively, or additional, we could lookup the oids at describe time (we describe results on explicit prepare and implicit redescribe). Unfortunately postgresql doesn't invalidated prepared queries when the sub-types change so we would either need to fallback to anonymous or crash if types change.

For decoding anonymous composites I am not sure. Technically if we find a type we don't know during decode we could look it up after results finish and then decode again. That'd be complex.

I feel this issue invalidates our general type handling approach somewhat and means edge cases will always exist. We may wish to consider having an additional type module that behaves like the majority of drivers in other languages: binary for built in types, text for other types. Of course we are making a trade off to get a richer type system so it might not be worthwhile changing existing behavior.

@michalmuskala
Copy link
Member

I wonder if it maybe would make sense to have some explicit API to tell postgrex that types have changed. I can see this being used when you migrate your database and then call some API in ecto to tell it about this. This could potentially give us some way of handling this - especially if we're saying we can never avoid all the edge cases.

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

No branches or pull requests

3 participants