lidavidm commented on code in PR #4141:
URL: https://github.com/apache/arrow-adbc/pull/4141#discussion_r2985620177
##########
rust/core/src/error.rs:
##########
@@ -102,10 +102,26 @@ impl Error {
impl Display for Error {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ let safe_ascii = |c: c_char| -> char {
+ if c == 0 {
+ '0'
+ } else if c >= 32 && c <= 126 {
+ char::from(c as u8)
+ } else {
+ '\u{FFFD}'
Review Comment:
Maybe we can still display the value somehow instead of hiding it? (Just
stringify the integer value instead?)
##########
rust/core/src/error.rs:
##########
@@ -102,10 +102,26 @@ impl Error {
impl Display for Error {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ let safe_ascii = |c: c_char| -> char {
+ if c == 0 {
+ '0'
+ } else if c >= 32 && c <= 126 {
+ char::from(c as u8)
+ } else {
+ '\u{FFFD}'
+ }
+ };
write!(
f,
- "{:?}: {} (sqlstate: {:?}, vendor_code: {})",
- self.status, self.message, self.sqlstate, self.vendor_code
+ "{:?}: {} (sqlstate: {}{}{}{}{}, vendor_code: {})",
Review Comment:
Maybe what'd be better is: if sqlstate is all-NUL, omit it; if all-ASCII,
stringify it; else fall back to the current representation?
##########
rust/core/src/error.rs:
##########
@@ -102,10 +102,26 @@ impl Error {
impl Display for Error {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ let safe_ascii = |c: c_char| -> char {
+ if c == 0 {
+ '0'
Review Comment:
I think I'd prefer something that isn't ambiguous with an actual sqlstate of
'00000'.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
