add support for numerics #1324
add support for numerics #1324
Conversation
eeeebbbbrrrr
left a comment
There was a problem hiding this comment.
Just a quick look, but this code is generally structured the way you gotta do it. It always feels wrong to enumerate a hardcoded list of type oids/names, but that's just kinda the nature of the beast with Rust and Postgres.
| "int8" => row[column.position].value::<i64>().unwrap().map(|v| v.to_string()), | ||
| "float4" => row[column.position].value::<f32>().unwrap().map(|v| v.to_string()), | ||
| "float8" => row[column.position].value::<f64>().unwrap().map(|v| v.to_string()), | ||
| "numeric" => row[column.position].value::<AnyNumeric>().unwrap().map(|v| v.to_string()), | ||
| "bpchar" | "text" | "varchar" => { |
There was a problem hiding this comment.
Hey! @montanalow asked me to take a quick look here.
I feel like these string types names would be better as OID values. pgrx has them all in pg_sys::, like pg_sys::INT8OID, pg_sys::NUMERICOID, pg_sys::NUMERICARRAYOID, and so on. It looks like this is the approach you use up in model.rs already.
I guess you'd have to figure out the type oids wherever you construct the Column entries, but that doesn't seem too difficult. I think in that SELECT statement around line 505 you could write udt_name::text::regtype::oid instead of udt_name::TEXT.
Comparing on Oid value will be a little more performant, I suppose, and it'll future proof you from accidentally making type-os in the code.
| _ => error!( | ||
| "Unhandled type for quantitative scalar column: {} {:?}", | ||
| column.name, column.pg_type |
There was a problem hiding this comment.
And now that you have the type oids, you could call its output function here and just get its string representation from Postgres. Who knows if it'd then be something you could otherwise handle, but maybe?
Same thing around line 1090 for the array case.
fix #1041