You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by al...@apache.org on 2022/11/16 18:34:24 UTC
[arrow-datafusion] branch master updated: Fix negative interval parsing bug (#4238)
This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git
The following commit(s) were added to refs/heads/master by this push:
new 905935cf2 Fix negative interval parsing bug (#4238)
905935cf2 is described below
commit 905935cf2eb90130c340d0a6512f66189319139e
Author: Jeffrey <22...@users.noreply.github.com>
AuthorDate: Thu Nov 17 05:34:19 2022 +1100
Fix negative interval parsing bug (#4238)
---
datafusion/common/src/parsers.rs | 56 +++++++++++++++++++++++++++++++++++++---
1 file changed, 52 insertions(+), 4 deletions(-)
diff --git a/datafusion/common/src/parsers.rs b/datafusion/common/src/parsers.rs
index 9016103d9..5d937a4ec 100644
--- a/datafusion/common/src/parsers.rs
+++ b/datafusion/common/src/parsers.rs
@@ -203,8 +203,11 @@ pub fn parse_interval(leading_field: &str, value: &str) -> Result<ScalarValue> {
if (result_nanos % 1_000_000 != 0)
|| (result_month != 0 && (result_days != 0 || result_nanos != 0))
{
- let result: i128 =
- ((result_month as i128) << 96) | ((result_days as i128) << 64) | result_nanos;
+ let result: i128 = ((result_month as i128) << 96)
+ // ensure discard high 32 bits of result_days before casting to i128
+ | (((result_days & u32::MAX as i64) as i128) << 64)
+ // ensure discard high 64 bits of result_nanos
+ | (result_nanos & u64::MAX as i128);
return Ok(ScalarValue::IntervalMonthDayNano(Some(result)));
}
@@ -215,7 +218,9 @@ pub fn parse_interval(leading_field: &str, value: &str) -> Result<ScalarValue> {
}
// IntervalMonthDayNano uses nanos, but IntervalDayTime uses millis
- let result: i64 = (result_days << 32) | (result_nanos as i64 / 1_000_000);
+ let result: i64 =
+ // ensure discard high 32 bits of milliseconds
+ (result_days << 32) | ((result_nanos as i64 / 1_000_000) & (u32::MAX as i64));
Ok(ScalarValue::IntervalDayTime(Some(result)))
}
@@ -249,6 +254,21 @@ mod test {
.to_string(),
r#"Invalid input syntax for type interval: "1 centurys 1 month""#
);
+
+ assert_eq!(
+ parse_interval("months", "3 year -1 month").unwrap(),
+ ScalarValue::new_interval_ym(3, -1)
+ );
+
+ assert_eq!(
+ parse_interval("months", "-3 year -1 month").unwrap(),
+ ScalarValue::new_interval_ym(-3, -1)
+ );
+
+ assert_eq!(
+ parse_interval("months", "-3 year 1 month").unwrap(),
+ ScalarValue::new_interval_ym(-3, 1)
+ );
}
#[test]
@@ -268,7 +288,25 @@ mod test {
assert_eq!(
parse_interval("months", "7 days 5 minutes").unwrap(),
- ScalarValue::new_interval_dt(7, (5.0 * 60.0 * MILLIS_PER_SECOND) as i32)
+ ScalarValue::new_interval_dt(7, 5 * 60 * MILLIS_PER_SECOND as i32)
+ );
+
+ assert_eq!(
+ parse_interval("months", "7 days -5 minutes").unwrap(),
+ ScalarValue::new_interval_dt(7, -5 * 60 * MILLIS_PER_SECOND as i32)
+ );
+
+ assert_eq!(
+ parse_interval("months", "-7 days 5 hours").unwrap(),
+ ScalarValue::new_interval_dt(-7, 5 * 60 * 60 * MILLIS_PER_SECOND as i32)
+ );
+
+ assert_eq!(
+ parse_interval("months", "-7 days -5 hours -5 minutes -5 seconds").unwrap(),
+ ScalarValue::new_interval_dt(
+ -7,
+ -(5 * 60 * 60 + 5 * 60 + 5) * MILLIS_PER_SECOND as i32
+ )
);
}
@@ -288,6 +326,16 @@ mod test {
parse_interval("months", "1 year 1 day 0.1 milliseconds").unwrap(),
ScalarValue::new_interval_mdn(12, 1, 1_00 * 1_000)
);
+
+ assert_eq!(
+ parse_interval("months", "1 month -1 second").unwrap(),
+ ScalarValue::new_interval_mdn(1, 0, -1_000_000_000)
+ );
+
+ assert_eq!(
+ parse_interval("months", "-1 year -1 month -1 week -1 day -1 hour -1 minute -1 second -1.11 millisecond").unwrap(),
+ ScalarValue::new_interval_mdn(-13, -8, -(60 * 60 + 60 + 1) * NANOS_PER_SECOND as i64 - 1_110_000)
+ );
}
#[test]