pgrx
pgrx copied to clipboard
pgcentralfoundation
External binary representation support (SEND/RECEIVE for custom types)
Problem: text type representation is not always efficient
For this reason, Postgres allows types to have an external binary representation. Also, some clients insist on using binary representation.
Solution: introduce SendRecvFuncs trait and sendrecvfuncs attribute
These are used to specify how external binary representation encoding is accomplished.
Several traits all reusing the same pattern suggests we should be attempting to create a more generic interface instead of copypasta.
I am not against having a "more generic" interface, but since we don't have any specific proposal for one, can we work on getting this one since it's of very practical use as is? Even if under a feature gate if so desired.
I've spent a good amount of time getting it done following the current approach found in the library (in/out type of traits), and it feels that saying "we should now find a common interface" is an unfair treatment of the PR. So I would propose to get it into a mergeable shape first and get it in. We can work on generalizing interfaces for these things as a follow-up. What do you think?
My concern is not necessarily a blocking concern, just something of note.
As discussed in private, I understand the concern of SendRecvFuncs superficial similarity to PgVarlenaInOutFuncs. That being said, while they are very similar shape-wise, their distinction lies in the specificity of such a trait (Send/Recv are intended explicitly for SEND/RECEIVE functions of the type).
PgVarlenaInOutFuncs is not suitable for some types (where Copy is not feasible), and thus I didn't consider adopting that trait (or breaking its API). I understand that you have doubts about whether Copy is required and whether it can be relaxed to Clone.
You suggested that perhaps in most cases users will not need different serialization (between varlena and send/receive). While I tend to agree, I don't think this should mean we should not make it possible to specify a send/receive-specific implementation.
I feel like this can be a very productive discussion that can help us design the next iteration of API and make this part of pgx much smoother. I'd love to be a part of the conversation.
My only concern is making external binary format (as a feature) depend on the timeline of this discussion. I suggest getting it done first (with the caveat of some potentially changing APIs; but we're pre 1.0 so we can afford some refining) and then, once there's a good consensus, transition to a better design.
Several traits all reusing the same pattern suggests we should be attempting to create a more generic interface instead of copypasta.
Yes
My concern is not necessarily a blocking concern, just something of note.
I think it probably should be. We're at the point where a better approach is necessary.
I've been thinking about this over the holiday and I'd like to take a stab at making all of this better. I have some ideas, but don't quite yet have the time to focus on it.
If you don't have time for this, perhaps it's a case of "better is the enemy of good," and we should consider something like this PR to provide the functionality first, and improve it later?
I don't quite have the time yet. I will soon and I'm not all that excited about adding some code to paper over design flaws now, just to remove it all in say, a week.
In practice, almost all implementations and users of PgVarlenaInOutFuncs are doing it via macro code, so I am not really worried about breaking users of it who manually touch it.
