- Notifications
You must be signed in to change notification settings - Fork328
add support for numerics#1324
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
eeeebbbbrrrr left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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" => { |
eeeebbbbrrrrFeb 22, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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 inpg_sys::, likepg_sys::INT8OID,pg_sys::NUMERICOID,pg_sys::NUMERICARRAYOID, and so on. It looks like this is the approach you use up inmodel.rs already.
I guess you'd have to figure out the type oids wherever you construct theColumn entries, but that doesn't seem too difficult. I think in that SELECT statement around line 505 you could writeudt_name::text::regtype::oid instead ofudt_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 |
eeeebbbbrrrrFeb 22, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
And now that you have the type oids, youcould 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