-
Notifications
You must be signed in to change notification settings - Fork 686
fix(mysql-cdc): up cast all unsigned int types #23278
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
case "bigint": | ||
return val == Data.DataType.TypeName.INT64_VALUE; | ||
return val == Data.DataType.TypeName.INT64_VALUE | ||
|| val == Data.DataType.TypeName.DECIMAL_VALUE; |
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.
I will try to distinguish signed bigint from unsigned bigint in a later PR to achieve more accurate schema check.
Err(_) => { | ||
// Only attempt base64 decoding in Precise mode | ||
match self.bigint_unsigned_handling { |
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.
Extract this special logic into a dedicated function, then we can do
-ScalarImpl::Decimal(Decimal::from_str(str).map_err(|_err| create_error())?)
+ScalarImpl::Decimal(Decimal::from_str(str).or_else(|_err| second_chance_base64(..))?)
This avoids deep indentation, and keeps the flow of normal handling uninterrupted when reading thru the code.
} | ||
DataType::Int16 => { | ||
handle_data_type!(mysql_row, mysql_datum_index, column_name, i16) | ||
handle_data_type_with_signed!(mysql_row, mysql_datum_index, column_name, i16, u16) |
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.
How does upcast work here?
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.
Could you explain the relationship between DataType::Int16, i16, and u16?
DataType::Date => Value::from(value.into_date().0), | ||
DataType::Time => Value::from(value.into_time().0), | ||
DataType::Timestamp => Value::from(value.into_timestamp().0), | ||
DataType::Decimal => Value::from(value.into_decimal().to_string()), |
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.
So we can also query mysql decimal or bigint unsigned with a string?
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.
I think so.
|
||
match Decimal::from_str(str_val) { | ||
Ok(decimal) => decimal.into(), | ||
Err(_) => { |
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.
When will Err occur?
} | ||
DataType::Int16 => { | ||
handle_data_type!(mysql_row, mysql_datum_index, column_name, i16) | ||
handle_data_type_with_signed!(mysql_row, mysql_datum_index, column_name, i16, u16) |
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.
Could you explain the relationship between DataType::Int16, i16, and u16?
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Allow up cast unsigned data type to signed data type (u16 -> i32, u32 -> i64, i64 -> decimal).
Checklist
Documentation
Release note