Add a default DynamicValue decoder for diesel_dynamic_schema - #5164
Add a default DynamicValue decoder for diesel_dynamic_schema#5164LucaCappelletti94 wants to merge 3 commits into
DynamicValue decoder for diesel_dynamic_schema#5164Conversation
ee86324 to
ed30fd8
Compare
| #[cfg(feature = "chrono")] | ||
| Timestamp(chrono::NaiveDateTime), | ||
| /// A UTC timestamp. | ||
| #[cfg(feature = "chrono")] | ||
| TimestampTz(chrono::DateTime<chrono::Utc>), | ||
| /// A calendar date. | ||
| #[cfg(feature = "chrono")] | ||
| Date(chrono::NaiveDate), | ||
| /// A PostgreSQL time of day. | ||
| #[cfg(feature = "chrono")] | ||
| Time(chrono::NaiveTime), | ||
| /// A MySQL or MariaDB `TIME` duration. | ||
| #[cfg(feature = "chrono")] | ||
| Duration(chrono::Duration), |
There was a problem hiding this comment.
My main concern with a feature like that is that it hard codes the rust side types. Some users might prefer different crates for date/time handling (and other use-cases as well!). That's now even more the case with chrono being soft-deprecated in favor of jiff. So by adding something like this to our public API we make it harder to add support for other crates like jiff later.
Now users could still write their own version of this as they currently need to do anyway, but choosing anything would signal an official promotion by diesel that this or that crate is the best fit there.
I'm not sure how to best resolve that problem, given that we would need to chose something in the end. That's the reason why we've not implemented this yet.
One "simple" solution would be to just not deal with types from third party crates here and fall back to to manually written enums for those users requiring them. The other solution would be to provide a derive that creates the underlying mapping code for the user based on their selection. I'm not sure if there are other possible solutions.
That's mostly a brain dump, not a complete review or request to change this to anything specific. Let's use this to start the discussion about what to do exactly.
There was a problem hiding this comment.
I was gnawing at this myself over the day, I think I have a possible "eat our cake and have it too" option. This code was fine in my downstream library, but here it needs to generalize and my idea would be to create a trait like "TheseAreAllTheTypes" with as associated types all of the above, such as "Date", then the enum takes a generic which implements that type. We might then provide a reasonable default, maybe if we really want to nail the unstability message home under the unstable API feature flag. Regarding what "reasonable" means I am not sure.
There was a problem hiding this comment.
I worked on it a bit more, my rough proposal (which I am still fleshing out to see whether it has any pain points) would be something like:
enum EnumWithAllTypes<Custom=()> {all ... of .. the .. core .. types, Custom(Custom)}And we dispatch from there, so no crate preference needs to be specified in the core, except possibly a "reasonable" ready to go variant.
def50fe to
ba3a42e
Compare
ba3a42e to
9b60a97
Compare
diesel_dynamic_schemacan select columns whose SQL types are only known at runtime, but decoding those rows previously required each caller to implementFromSql<Any, DB>and repeat the backend dispatch.This adds a metadata pass alongside SQL generation.
collect_output_metadatawalks the selected expressions in rendered order. Dynamic columns record their declared SQL type and direct column origin, while tuples, boxed queries, select statements, andRETURNINGclauses forward that metadata. The dynamic loader collects it before execution, checks that it matches the returned field count, and combines each entry with the field name and backend type tag.DynamicValueBackenduses that context to select Diesel's existing typedFromSqlimplementations. Shared values becomeNull,Bool,Integer,Unsigned,Float,Text, orBytes. Values without a backend-neutral representation remain in backend-specific variants.PostgreSQLdispatches on OIDs and recursively decodes arrays while preserving dimensions and lower bounds.SQLitecombines its storage class with the declared SQL type.MySQLandMariaDBdispatch on their type tags and preserve values such asMysqlTime, including negativeTIMEdurations.I have omitted smaller integers and floats such as
u8since the enum size will be the same for all variants, so it did not seem worth it.DynamicValueExtensionruns before the default decoder and receives the declared type, backend tag, field name, direct column origin, andPostgreSQLarray subscripts. This lets callers and the included optional integrations decodechrono,time,bigdecimal,uuid, JSON, and network address values without adding those representations to the core enum.The loading API covers named and positional rows, vectors and iterators, and single or multiple results from
SELECTandRETURNING. The existingDynamicRowand customFromSql<Any, DB>path remains available.This PR is "stacked" on #5170 and #5167.
This is the first PR in the runtime-schema
DMLseries. Follow-ups add query-source validation, shared mutation extension points, and dynamic insert, update, and delete.The new dependency versions are declared locally in this crate for now. I think they should move to the workspace dependency declarations before this lands to prevent version drift.