luoyuxia commented on code in PR #175:
URL: https://github.com/apache/fluss-rust/pull/175#discussion_r2704157911
##########
crates/fluss/src/metadata/datatype.rs:
##########
@@ -457,12 +457,43 @@ impl DecimalType {
Self::with_nullable(true, precision, scale)
}
- pub fn with_nullable(nullable: bool, precision: u32, scale: u32) -> Self {
- DecimalType {
+ /// Try to create a DecimalType with validation, returning an error if
parameters are invalid.
+ pub fn try_with_nullable(nullable: bool, precision: u32, scale: u32) ->
Result<Self> {
+ // Validate precision
+ if !(Self::MIN_PRECISION..=Self::MAX_PRECISION).contains(&precision) {
+ return Err(IllegalArgument {
+ message: format!(
+ "Decimal precision must be between {} and {} (both
inclusive), got: {}",
+ Self::MIN_PRECISION,
+ Self::MAX_PRECISION,
+ precision
+ ),
+ });
+ }
+ // Validate scale
+ // Note: MIN_SCALE is 0, and scale is u32, so scale >= MIN_SCALE is
always true
+ if scale > precision {
+ return Err(IllegalArgument {
+ message: format!(
+ "Decimal scale must be between {} and the precision {}
(both inclusive), got: {}",
+ Self::MIN_SCALE,
+ precision,
+ scale
+ ),
+ });
+ }
+ Ok(DecimalType {
nullable,
precision,
scale,
- }
+ })
+ }
+
+ /// Create a DecimalType with validation, panicking if parameters are
invalid.
+ /// Use this for hardcoded schemas where validation failure is a
programming error.
+ pub fn with_nullable(nullable: bool, precision: u32, scale: u32) -> Self {
Review Comment:
nit: how about remove `with_nullable` and rename `try_with_nullable` to
`with_nullable` since in the future we will all migrate to return `Result`
instead of panice directly.
##########
crates/fluss/src/metadata/datatype.rs:
##########
@@ -457,12 +457,43 @@ impl DecimalType {
Self::with_nullable(true, precision, scale)
}
- pub fn with_nullable(nullable: bool, precision: u32, scale: u32) -> Self {
- DecimalType {
+ /// Try to create a DecimalType with validation, returning an error if
parameters are invalid.
+ pub fn try_with_nullable(nullable: bool, precision: u32, scale: u32) ->
Result<Self> {
+ // Validate precision
+ if !(Self::MIN_PRECISION..=Self::MAX_PRECISION).contains(&precision) {
+ return Err(IllegalArgument {
+ message: format!(
+ "Decimal precision must be between {} and {} (both
inclusive), got: {}",
+ Self::MIN_PRECISION,
+ Self::MAX_PRECISION,
+ precision
+ ),
+ });
+ }
+ // Validate scale
+ // Note: MIN_SCALE is 0, and scale is u32, so scale >= MIN_SCALE is
always true
+ if scale > precision {
+ return Err(IllegalArgument {
+ message: format!(
+ "Decimal scale must be between {} and the precision {}
(both inclusive), got: {}",
+ Self::MIN_SCALE,
+ precision,
+ scale
+ ),
+ });
+ }
+ Ok(DecimalType {
nullable,
precision,
scale,
- }
+ })
+ }
+
+ /// Create a DecimalType with validation, panicking if parameters are
invalid.
+ /// Use this for hardcoded schemas where validation failure is a
programming error.
+ pub fn with_nullable(nullable: bool, precision: u32, scale: u32) -> Self {
Review Comment:
The same to other datatypes in this pr
--
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]
