fix(query): check_timestamp/date -> clamp_timestamp/date (#16634)

If Date/Timestamp is invalide, convert it to Date/TimestampMIN
databendlabs · Oct 21, 2024 · 7a6f1fb · 7a6f1fb
1 parent b1538fa
commit 7a6f1fb
Show file tree

Hide file tree

Showing 12 changed files with 377 additions and 401 deletions.
diff --git a/src/query/expression/src/types/date.rs b/src/query/expression/src/types/date.rs
@@ -23,6 +23,7 @@ use databend_common_arrow::arrow::buffer::Buffer;
 use databend_common_exception::ErrorCode;
 use databend_common_io::cursor_ext::BufferReadDateTimeExt;
 use databend_common_io::cursor_ext::ReadBytesExt;
+use log::error;
 use num_traits::AsPrimitive;
 
 use super::number::SimpleDomain;
@@ -46,12 +47,14 @@ pub const DATE_MIN: i32 = -354285;
 pub const DATE_MAX: i32 = 2932896;
 
 /// Check if date is within range.
+/// /// If days is invalid convert to DATE_MIN.
 #[inline]
-pub fn check_date(days: i64) -> Result<i32, String> {
+pub fn clamp_date(days: i64) -> i32 {
     if (DATE_MIN as i64..=DATE_MAX as i64).contains(&days) {
-        Ok(days as i32)
+        days as i32
     } else {
-        Err("date is out of range".to_string())
+        error!("{}", format!("date {} is out of range", days));
+        DATE_MIN
     }
 }
 

diff --git a/src/query/expression/src/types/timestamp.rs b/src/query/expression/src/types/timestamp.rs
@@ -24,6 +24,7 @@ use databend_common_exception::ErrorCode;
 use databend_common_io::cursor_ext::BufferReadDateTimeExt;
 use databend_common_io::cursor_ext::DateTimeResType;
 use databend_common_io::cursor_ext::ReadBytesExt;
+use log::error;
 
 use super::number::SimpleDomain;
 use crate::property::Domain;
@@ -53,12 +54,12 @@ pub const PRECISION_MILLI: u8 = 3;
 pub const PRECISION_SEC: u8 = 0;
 
 /// Check if the timestamp value is valid.
+/// If timestamp is invalid convert to TIMESTAMP_MIN.
 #[inline]
-pub fn check_timestamp(micros: i64) -> Result<i64, String> {
-    if (TIMESTAMP_MIN..=TIMESTAMP_MAX).contains(&micros) {
-        Ok(micros)
-    } else {
-        Err("timestamp is out of range".to_string())
+pub fn clamp_timestamp(micros: &mut i64) {
+    if !(TIMESTAMP_MIN..=TIMESTAMP_MAX).contains(micros) {
+        error!("{}", format!("timestamp {} is out of range", micros));
+        *micros = TIMESTAMP_MIN;
     }
 }
 

diff --git a/src/query/expression/src/utils/date_helper.rs b/src/query/expression/src/utils/date_helper.rs
@@ -32,8 +32,8 @@ use databend_common_exception::Result;
 use databend_common_io::cursor_ext::unwrap_local_time;
 use num_traits::AsPrimitive;
 
-use crate::types::date::check_date;
-use crate::types::timestamp::check_timestamp;
+use crate::types::date::clamp_date;
+use crate::types::timestamp::clamp_timestamp;
 use crate::types::timestamp::MICROS_PER_SEC;
 
 #[derive(Debug, Clone, Copy)]
@@ -341,11 +341,11 @@ macro_rules! impl_interval_year_month {
             ) -> std::result::Result<i32, String> {
                 let date = date.to_date(tz.tz);
                 let new_date = $op(date.year(), date.month(), date.day(), delta.as_())?;
-                check_date(
+                Ok(clamp_date(
                     new_date
                         .signed_duration_since(NaiveDate::from_ymd_opt(1970, 1, 1).unwrap())
                         .num_days(),
-                )
+                ))
             }
 
             pub fn eval_timestamp(
@@ -355,11 +355,11 @@ macro_rules! impl_interval_year_month {
             ) -> std::result::Result<i64, String> {
                 let ts = us.to_timestamp(tz.tz);
                 let new_ts = $op(ts.year(), ts.month(), ts.day(), delta.as_())?;
-                check_timestamp(
-                    NaiveDateTime::new(new_ts, ts.time())
-                        .and_utc()
-                        .timestamp_micros(),
-                )
+                let mut ts = NaiveDateTime::new(new_ts, ts.time())
+                    .and_utc()
+                    .timestamp_micros();
+                clamp_timestamp(&mut ts);
+                Ok(ts)
             }
         }
     };
@@ -485,19 +485,18 @@ impl EvalWeeksImpl {
 pub struct EvalDaysImpl;
 
 impl EvalDaysImpl {
-    pub fn eval_date(date: i32, delta: impl AsPrimitive<i64>) -> std::result::Result<i32, String> {
-        check_date((date as i64).wrapping_add(delta.as_()))
+    pub fn eval_date(date: i32, delta: impl AsPrimitive<i64>) -> i32 {
+        clamp_date((date as i64).wrapping_add(delta.as_()))
     }
 
     pub fn eval_date_diff(date_start: i32, date_end: i32) -> i32 {
         date_end - date_start
     }
 
-    pub fn eval_timestamp(
-        date: i64,
-        delta: impl AsPrimitive<i64>,
-    ) -> std::result::Result<i64, String> {
-        check_timestamp(date.wrapping_add(delta.as_() * MICROSECS_PER_DAY))
+    pub fn eval_timestamp(date: i64, delta: impl AsPrimitive<i64>) -> i64 {
+        let mut value = date.wrapping_add(delta.as_() * MICROSECS_PER_DAY);
+        clamp_timestamp(&mut value);
+        value
     }
 
     pub fn eval_timestamp_diff(date_start: i64, date_end: i64) -> i64 {
@@ -511,22 +510,16 @@ impl EvalDaysImpl {
 pub struct EvalTimesImpl;
 
 impl EvalTimesImpl {
-    pub fn eval_date(
-        date: i32,
-        delta: impl AsPrimitive<i64>,
-        factor: i64,
-    ) -> std::result::Result<i32, String> {
-        check_date(
+    pub fn eval_date(date: i32, delta: impl AsPrimitive<i64>, factor: i64) -> i32 {
+        clamp_date(
             (date as i64 * MICROSECS_PER_DAY).wrapping_add(delta.as_() * factor * MICROS_PER_SEC),
         )
     }
 
-    pub fn eval_timestamp(
-        us: i64,
-        delta: impl AsPrimitive<i64>,
-        factor: i64,
-    ) -> std::result::Result<i64, String> {
-        check_timestamp(us.wrapping_add(delta.as_() * factor * MICROS_PER_SEC))
+    pub fn eval_timestamp(us: i64, delta: impl AsPrimitive<i64>, factor: i64) -> i64 {
+        let mut ts = us.wrapping_add(delta.as_() * factor * MICROS_PER_SEC);
+        clamp_timestamp(&mut ts);
+        ts
     }
 
     pub fn eval_timestamp_diff(date_start: i64, date_end: i64, factor: i64) -> i64 {

diff --git a/src/query/expression/src/values.rs b/src/query/expression/src/values.rs
@@ -78,7 +78,7 @@ use crate::types::number::F64;
 use crate::types::string::StringColumn;
 use crate::types::string::StringColumnBuilder;
 use crate::types::string::StringDomain;
-use crate::types::timestamp::check_timestamp;
+use crate::types::timestamp::clamp_timestamp;
 use crate::types::timestamp::TIMESTAMP_MAX;
 use crate::types::timestamp::TIMESTAMP_MIN;
 use crate::types::variant::JSONB_NULL;
@@ -2055,8 +2055,8 @@ impl ColumnBuilder {
                 string::CheckUTF8::check_utf8(&(&builder.data[last..last + offset])).unwrap();
             }
             ColumnBuilder::Timestamp(builder) => {
-                let value: i64 = reader.read_scalar()?;
-                check_timestamp(value)?;
+                let mut value: i64 = reader.read_scalar()?;
+                clamp_timestamp(&mut value);
                 builder.push(value);
             }
             ColumnBuilder::Date(builder) => {
@@ -2157,8 +2157,8 @@ impl ColumnBuilder {
             ColumnBuilder::Timestamp(builder) => {
                 for row in 0..rows {
                     let mut reader = &reader[step * row..];
-                    let value: i64 = reader.read_scalar()?;
-                    check_timestamp(value)?;
+                    let mut value: i64 = reader.read_scalar()?;
+                    clamp_timestamp(&mut value);
                     builder.push(value);
                 }
             }

diff --git a/src/query/formats/src/field_decoder/fast_values.rs b/src/query/formats/src/field_decoder/fast_values.rs
@@ -29,14 +29,14 @@ use databend_common_expression::serialize::read_decimal_with_size;
 use databend_common_expression::serialize::uniform_date;
 use databend_common_expression::types::array::ArrayColumnBuilder;
 use databend_common_expression::types::binary::BinaryColumnBuilder;
-use databend_common_expression::types::date::check_date;
+use databend_common_expression::types::date::clamp_date;
 use databend_common_expression::types::decimal::Decimal;
 use databend_common_expression::types::decimal::DecimalColumnBuilder;
 use databend_common_expression::types::decimal::DecimalSize;
 use databend_common_expression::types::nullable::NullableColumnBuilder;
 use databend_common_expression::types::number::Number;
 use databend_common_expression::types::string::StringColumnBuilder;
-use databend_common_expression::types::timestamp::check_timestamp;
+use databend_common_expression::types::timestamp::clamp_timestamp;
 use databend_common_expression::types::AnyType;
 use databend_common_expression::types::NumberColumnBuilder;
 use databend_common_expression::with_decimal_type;
@@ -289,8 +289,7 @@ impl FastFieldDecoderValues {
             self.common_settings().enable_dst_hour_fix,
         )?;
         let days = uniform_date(date);
-        check_date(days as i64)?;
-        column.push(days);
+        column.push(clamp_date(days as i64));
         Ok(())
     }
 
@@ -319,8 +318,8 @@ impl FastFieldDecoderValues {
                     );
                     return Err(ErrorCode::BadBytes(msg));
                 }
-                let micros = ts.timestamp_micros();
-                check_timestamp(micros)?;
+                let mut micros = ts.timestamp_micros();
+                clamp_timestamp(&mut micros);
                 column.push(micros.as_());
             }
             _ => unreachable!(),

diff --git a/src/query/formats/src/field_decoder/json_ast.rs b/src/query/formats/src/field_decoder/json_ast.rs
@@ -23,14 +23,14 @@ use databend_common_expression::serialize::read_decimal_from_json;
 use databend_common_expression::serialize::uniform_date;
 use databend_common_expression::types::array::ArrayColumnBuilder;
 use databend_common_expression::types::binary::BinaryColumnBuilder;
-use databend_common_expression::types::date::check_date;
+use databend_common_expression::types::date::clamp_date;
 use databend_common_expression::types::decimal::Decimal;
 use databend_common_expression::types::decimal::DecimalColumnBuilder;
 use databend_common_expression::types::decimal::DecimalSize;
 use databend_common_expression::types::nullable::NullableColumnBuilder;
 use databend_common_expression::types::number::Number;
 use databend_common_expression::types::string::StringColumnBuilder;
-use databend_common_expression::types::timestamp::check_timestamp;
+use databend_common_expression::types::timestamp::clamp_timestamp;
 use databend_common_expression::types::AnyType;
 use databend_common_expression::types::NumberColumnBuilder;
 use databend_common_expression::with_decimal_type;
@@ -266,14 +266,12 @@ impl FieldJsonAstDecoder {
                 let mut reader = Cursor::new(v.as_bytes());
                 let date = reader.read_date_text(&self.timezone, self.enable_dst_hour_fix)?;
                 let days = uniform_date(date);
-                check_date(days as i64)?;
-                column.push(days);
+                column.push(clamp_date(days as i64));
                 Ok(())
             }
             Value::Number(number) => match number.as_i64() {
                 Some(n) => {
-                    let n = check_date(n)?;
-                    column.push(n);
+                    column.push(clamp_date(n));
                     Ok(())
                 }
                 None => Err(ErrorCode::BadArguments("Incorrect date value")),
@@ -292,17 +290,17 @@ impl FieldJsonAstDecoder {
 
                 match ts {
                     DateTimeResType::Datetime(ts) => {
-                        let micros = ts.timestamp_micros();
-                        check_timestamp(micros)?;
+                        let mut micros = ts.timestamp_micros();
+                        clamp_timestamp(&mut micros);
                         column.push(micros.as_());
                     }
                     _ => unreachable!(),
                 }
                 Ok(())
             }
             Value::Number(number) => match number.as_i64() {
-                Some(n) => {
-                    check_timestamp(n)?;
+                Some(mut n) => {
+                    clamp_timestamp(&mut n);
                     column.push(n);
                     Ok(())
                 }

diff --git a/src/query/formats/src/field_decoder/nested.rs b/src/query/formats/src/field_decoder/nested.rs
@@ -24,14 +24,14 @@ use databend_common_expression::serialize::read_decimal_with_size;
 use databend_common_expression::serialize::uniform_date;
 use databend_common_expression::types::array::ArrayColumnBuilder;
 use databend_common_expression::types::binary::BinaryColumnBuilder;
-use databend_common_expression::types::date::check_date;
+use databend_common_expression::types::date::clamp_date;
 use databend_common_expression::types::decimal::Decimal;
 use databend_common_expression::types::decimal::DecimalColumnBuilder;
 use databend_common_expression::types::decimal::DecimalSize;
 use databend_common_expression::types::nullable::NullableColumnBuilder;
 use databend_common_expression::types::number::Number;
 use databend_common_expression::types::string::StringColumnBuilder;
-use databend_common_expression::types::timestamp::check_timestamp;
+use databend_common_expression::types::timestamp::clamp_timestamp;
 use databend_common_expression::types::AnyType;
 use databend_common_expression::types::NumberColumnBuilder;
 use databend_common_expression::with_decimal_type;
@@ -249,8 +249,7 @@ impl NestedValues {
             self.common_settings().enable_dst_hour_fix,
         )?;
         let days = uniform_date(date);
-        check_date(days as i64)?;
-        column.push(days);
+        column.push(clamp_date(days as i64));
         Ok(())
     }
 
@@ -262,7 +261,7 @@ impl NestedValues {
         let mut buf = Vec::new();
         self.read_string_inner(reader, &mut buf)?;
         let mut buffer_readr = Cursor::new(&buf);
-        let ts = if !buf.contains(&b'-') {
+        let mut ts = if !buf.contains(&b'-') {
             buffer_readr.read_num_text_exact()?
         } else {
             let t = buffer_readr.read_timestamp_text(
@@ -286,7 +285,7 @@ impl NestedValues {
                 _ => unreachable!(),
             }
         };
-        check_timestamp(ts)?;
+        clamp_timestamp(&mut ts);
         column.push(ts);
         Ok(())
     }

diff --git a/src/query/formats/src/field_decoder/separated_text.rs b/src/query/formats/src/field_decoder/separated_text.rs
@@ -23,12 +23,12 @@ use databend_common_expression::serialize::read_decimal_with_size;
 use databend_common_expression::serialize::uniform_date;
 use databend_common_expression::types::array::ArrayColumnBuilder;
 use databend_common_expression::types::binary::BinaryColumnBuilder;
-use databend_common_expression::types::date::check_date;
+use databend_common_expression::types::date::clamp_date;
 use databend_common_expression::types::decimal::Decimal;
 use databend_common_expression::types::decimal::DecimalColumnBuilder;
 use databend_common_expression::types::decimal::DecimalSize;
 use databend_common_expression::types::nullable::NullableColumnBuilder;
-use databend_common_expression::types::timestamp::check_timestamp;
+use databend_common_expression::types::timestamp::clamp_timestamp;
 use databend_common_expression::types::AnyType;
 use databend_common_expression::types::Number;
 use databend_common_expression::types::NumberColumnBuilder;
@@ -257,13 +257,12 @@ impl SeparatedTextDecoder {
             self.common_settings().enable_dst_hour_fix,
         )?;
         let days = uniform_date(date);
-        check_date(days as i64)?;
-        column.push(days);
+        column.push(clamp_date(days as i64));
         Ok(())
     }
 
     fn read_timestamp(&self, column: &mut Vec<i64>, data: &[u8]) -> Result<()> {
-        let ts = if !data.contains(&b'-') {
+        let mut ts = if !data.contains(&b'-') {
             read_num_text_exact(data)?
         } else {
             let mut buffer_readr = Cursor::new(&data);
@@ -288,7 +287,7 @@ impl SeparatedTextDecoder {
                 _ => unreachable!(),
             }
         };
-        check_timestamp(ts)?;
+        clamp_timestamp(&mut ts);
         column.push(ts);
         Ok(())
     }