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

value decoding (and encoding) needs additional access #346

Open
isoos opened this issue Aug 20, 2024 · 5 comments
Open

value decoding (and encoding) needs additional access #346

isoos opened this issue Aug 20, 2024 · 5 comments

Comments

@isoos
Copy link
Owner

isoos commented Aug 20, 2024

My hesitance to go ahead with #342 and also with #343 has the same root: in their current form, they seem to be adding non-trivial complexity that could become barriers for the future improvements, while in the ideal world such features should be added with much less friction and much less interconnectedness.

After a some thinking and high-level fiddling, I think I have found a way forward though: we will need to refactor the value encoding/decoding to have access to:

  • connection settings (like client-provided timezone)
  • runtime parameter values (like server-provided timezone)
  • the TypeRegistry itself (there should be no need to use any other reference than the one provided in the connection settings)
  • the connection or ways to open a new connection + an OID cache that can use it to query yet-unknown identifiers (this becomes tricky in certain clusters so it shouldn't be 100% automatic caching)
  • user-provided transformation functions for specific types - e.g. to pull in larger timezone databases like pg_timezone, which may not be needed for all users

I'm not entirely sure how such code would look like, but I think the above feature would now only allow the two pending features to go ahead, but also allow better overall extensibility.

/cc @insinfo @wolframm

@isoos
Copy link
Owner Author

isoos commented Aug 20, 2024

I think the OID cache part has a clean separation from the rest. The other big change would be the decoding becoming async/FutureOr because of the potential need for OID type description query. It seems to be not breaking though.

@isoos
Copy link
Owner Author

isoos commented Sep 5, 2024

I think this is now implemented, alongside with custom codec registration.

/cc @insinfo @wolframm: I've released a 3.4.0-dev.1 prerelease version with it, I think the API is good enough now, and you can go ahead with the PRs. Please let me know if there is something missing or in conflict with the goals.

@isoos
Copy link
Owner Author

isoos commented Sep 13, 2024

Note: I've released 3.4.0-dev.2 with now with likely future-proofed async-enabled codec API (+ related message processing). Please, if possible, test and give me feedback before committing to a final version on it.

@insinfo
Copy link

insinfo commented Sep 14, 2024

Hi, I had an accident and will be away from programming for a while. It would be great if you could implement timezone support based on my implementation with the modifications you introduced. Congratulations on your work.

@isoos
Copy link
Owner Author

isoos commented Sep 17, 2024

@insinfo: I'm planning to help you out with the implementation, however I won't have the bandwidth for that in the next week or so. Because of that I've published 3.4.0 as-is, and we shall figure out the details for your use-case later, I'm rather optimistic that everything need is in it.

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

No branches or pull requests

2 participants