How useful is this really on its own without the ability to retrieve results from the database in the text format?

How useful is this really on its own without the ability to retrieve results from the database in the text format?

I am not sure i fully understand the question. Are you referring to using the simple query protocol from pg?
In my case i construct queries that always return::text columns while the parameters are of different types.

My motivation (and it seems@NAlexPear too) is to let the pg handle type inference.
A few reasons in no particular order:

• In some case, one does not always work with a "known" schema so the parameter types are not trivial to determine at query creation time (think of products likeHasura) in which case it's very helpful to send over anunknown and let pg determine the type of$1 and do the conversion. This becomes really problematic when the target database uses custom types(enums) so you cant have (i think) at runtime code that handles these situation as it seems in this lib custom types need to have predefined structs for them that deriveToSql
• Some types (mostly from extensions) do not support binary encoding but they all provide a mechanism to cast an unknown literal which would mean with this small addition one can use all the custom types directly. In some cases probably, as you mentioned somewhere, a double cast fromTEXT can be used though maybe not all types support that? and anyway$1 looks better then$1::MONEY::TEXT :) )
• Meta reason: pg protocol allows for such a use-case (sending over some params as text encoded), they probably have better reasons then mine as to why this functionality exists, so why not let the client library also support this event if we can't come up with very compelling use-cases (if it were a big refactor i would also be suspicious, and pushing back against bloat is also a valid reason, but it seems to be a minimal change with no obvious downsides and some potential upsides)

@NAlexPear do you have other reasons why having the ability to send over some params as text encoded is needed (and the current lib can not handle those usecases)?

ps:@sfackler thank you for the feedback and for considering this.

I've refactored theUnknown struct per your suggestion so now it's more of a convenience feature (which can be removed from the lib since the user can define it himself if needed).

The core functionality/flexibility can be provided only by having thefn encode_format(&self) -> i16 { 1 } function in the trait
and the line

let (param_formats, params):(Vec<_>, Vec<_>) = params.into_iter().map(|p| (p.borrow_to_sql().encode_format(),p)).unzip();

that takes into the consideration the output of this function when encoding parameters.

Thanks for taking a crack at this@ruslantalpa!

do you have other reasons why having the ability to send over some params as text encoded is needed (and the current lib can not handle those usecases)?

The ability to let Postgres infer types is the big one, as you've already pointed out, as well as the ability to support parameter types that don't have easy or intuitive Rust implementations. But I don't think that was the thrust of@sfackler's question when they asked:

How useful is this really on its own without the ability to retrieve results from the database in the text format?

I think the answer here is "still useful" for the type inference case, but only "marginally useful" for the type pass-through case. The most common use for those pass-through types would be as column types, and rare is the column that's modified but never read again.

Given those cases, it would make the most sense to me to allow for an interface that handles both inputsand outputs, since the latter often (but not always) depends on the former. Whether type inference of inputs on its own is a worth feature would be up to@sfackler, but it wouldn't cover my usecases on its own.

@NAlexPear do i understand correctly that in your usecase you also need to be able to read some columns where the pg type of that column does not support binary encoding? If that is the case maybe this might be an automated way to support them:

Amend thetypes::Type struct with a field calledsupports_binary_encoding and have that default totrue but when it gets to a situation where we encounter anoid that is not predefinedhere have theTYPEINFO_QUERY query also fetch thetypreceive andtypsend fields and based on them set thesupports_binary_encoding field of the type.

Now, since we have the binary encoding support stored in theType struct, it's possible now to selectively request text encoding for the columns in theencode_bind function fromquery.rs

With this setup it think it makes it possible for a user to defineToSql andFromSql traits for types that require sending and receiving columns encoded in text mode.

Would that cover the usecase?

Checking if the type has binary conversion functions is an interesting idea! It's not perfect (e.g. if you upgrade an extension and it adds a binary format that'd break your ability to read values), but it seems like it's probably the best we can do without enormous changes to how the query API works.

I think we'd want to allowToSql to pick if it's using text or binary format andFromSql would be forced to use binary unless the type only supports text mode.

Actually i think the core of the idea is not necessarily the "auto" part but it's that the Type has info about the the encoding it wants.

For now, the type detection first considers the ones implemented in the lib and after the ones attached to the clienthere but what if it was the other way around? This way you can "replace" the default type implementations dynamically by registering your own (which incidentally can request text encoding even if the default lib mode is in binary).

Anyway, leaving my comment about the order aside (can be considered later) do you want me to add in this PR the implementation for thesupports_binary_encoding idea and the changes to theTYPEINFO_QUERY (along with some tests and documentation)?

Let's leave that to a separate PR.

For this one, I think we just need encode_format to return an enum.

I'm also not sold on the nameUnknown. It may be best to not add a type like that at all to this library for now?

@sfackler sorry for the delay,
I removed theUnknown part and added theFormat enum (thank you@NAlexPear )

@sfackler is this PR ok as it is or do you want me to make some changes or it's not something you want to merge (no rush, whenever you have time)?
Thank you

Merged - sorry for the delay!