Re: [PR] fix: The inconsistency between scalar and array on the cast decimal to timestamp [datafusion]
findepi merged PR #16539: URL: https://github.com/apache/datafusion/pull/16539 -- 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] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] fix: The inconsistency between scalar and array on the cast decimal to timestamp [datafusion]
alamb commented on code in PR #16539:
URL: https://github.com/apache/datafusion/pull/16539#discussion_r2216741569
##
datafusion/common/src/scalar/mod.rs:
##
@@ -3075,38 +3075,7 @@ impl ScalarValue {
target_type: &DataType,
cast_options: &CastOptions<'static>,
) -> Result {
-let scalar_array = match (self, target_type) {
-(
-ScalarValue::Decimal128(Some(decimal_value), _, scale),
-DataType::Timestamp(time_unit, None),
-) => {
-let scale_factor = 10_i128.pow(*scale as u32);
-let seconds = decimal_value / scale_factor;
-let fraction = decimal_value % scale_factor;
-
-let timestamp_value = match time_unit {
-TimeUnit::Second => ScalarValue::Int64(Some(seconds as
i64)),
-TimeUnit::Millisecond => {
-let millis = seconds * 1_000 + (fraction * 1_000) /
scale_factor;
-ScalarValue::Int64(Some(millis as i64))
-}
-TimeUnit::Microsecond => {
-let micros =
-seconds * 1_000_000 + (fraction * 1_000_000) /
scale_factor;
-ScalarValue::Int64(Some(micros as i64))
-}
-TimeUnit::Nanosecond => {
-let nanos = seconds * 1_000_000_000
-+ (fraction * 1_000_000_000) / scale_factor;
-ScalarValue::Int64(Some(nanos as i64))
-}
-};
-
-timestamp_value.to_array()?
-}
-_ => self.to_array()?,
-};
-
+let scalar_array = self.to_array()?;
Review Comment:
that is pretty nice fixing bugs by deleting code 🏆
--
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]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] fix: The inconsistency between scalar and array on the cast decimal to timestamp [datafusion]
findepi commented on PR #16539: URL: https://github.com/apache/datafusion/pull/16539#issuecomment-3088779085 @alamb PTAL -- 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] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] fix: The inconsistency between scalar and array on the cast decimal to timestamp [datafusion]
findepi commented on PR #16539: URL: https://github.com/apache/datafusion/pull/16539#issuecomment-3085264189 > What is the status of this PR? Shall we merge it? Or are there outstanding issues to resolve? requires an update -- https://github.com/apache/datafusion/pull/16539#discussion_r2180903190 -- 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] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] fix: The inconsistency between scalar and array on the cast decimal to timestamp [datafusion]
findepi commented on code in PR #16539:
URL: https://github.com/apache/datafusion/pull/16539#discussion_r2214149519
##
datafusion/common/src/scalar/mod.rs:
##
@@ -3069,7 +3069,14 @@ impl ScalarValue {
ScalarValue::Decimal128(Some(decimal_value), _, scale),
DataType::Timestamp(time_unit, None),
Review Comment:
https://github.com/apache/datafusion/pull/16639 is now merged.
please remove the whole `let scalar_array = match (self, target_type) { ...
}` block, replacing it with just `self.to_array()?`.
--
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]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] fix: The inconsistency between scalar and array on the cast decimal to timestamp [datafusion]
alamb commented on PR #16539: URL: https://github.com/apache/datafusion/pull/16539#issuecomment-3070587452 What is the status of this PR? Shall we merge it? Or are there outstanding issues to resolve? -- 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] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] fix: The inconsistency between scalar and array on the cast decimal to timestamp [datafusion]
findepi commented on code in PR #16539:
URL: https://github.com/apache/datafusion/pull/16539#discussion_r2180903190
##
datafusion/common/src/scalar/mod.rs:
##
@@ -3069,7 +3069,14 @@ impl ScalarValue {
ScalarValue::Decimal128(Some(decimal_value), _, scale),
DataType::Timestamp(time_unit, None),
Review Comment:
In https://github.com/apache/datafusion/pull/16639 i tried to clear the path
for more changes here.
If and once that one merged, we should be able to remove this whole code
block.
--
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]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
