Skip to content

Commit 828002d

Browse files
committed
fix offset clamp
1 parent a5d9226 commit 828002d

18 files changed

Lines changed: 968 additions & 845 deletions

File tree

src/common/io/src/cursor_ext/cursor_read_datetime_ext.rs

Lines changed: 8 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ use databend_common_timezone::fast_utc_from_local;
2727

2828
use crate::cursor_ext::cursor_read_bytes_ext::ReadBytesExt;
2929
use crate::datetime::check_input_year;
30+
use crate::datetime::check_timezone_offset;
3031
use crate::datetime::parse_standard_timestamp as parse_iso_timestamp;
3132

3233
pub enum DateTimeResType {
@@ -44,9 +45,6 @@ const DATE_LEN: usize = 10;
4445
const MICROS_PER_SEC: i64 = 1_000_000;
4546
const SECONDS_PER_DAY: i64 = 86_400;
4647

47-
// ISO 8601 maximum offset.
48-
const MAX_OFFSET_HOURS: i32 = 14;
49-
5048
fn parse_time_part(buf: &[u8], size: usize) -> Result<u32> {
5149
if size > 0 && size < 3 {
5250
Ok(lexical_core::FromLexical::from_lexical(buf)
@@ -195,18 +193,15 @@ fn read_offset_seconds<T: AsRef<[u8]>>(
195193
west_tz: bool,
196194
) -> Result<i32> {
197195
fn validated(hour_offset: i32, minute_offset: i32, west_tz: bool) -> Result<i32> {
198-
let in_range = (hour_offset == MAX_OFFSET_HOURS && minute_offset == 0)
199-
|| ((0..60).contains(&minute_offset) && hour_offset < MAX_OFFSET_HOURS);
200-
201-
if !in_range {
202-
return Err(ErrorCode::BadBytes(format!(
203-
"Invalid Timezone Offset: The minute offset '{}' is outside the valid range. Expected range is [00-59] within a timezone gap of [-14:00, +14:00]",
204-
minute_offset
205-
)));
196+
if !(0..60).contains(&minute_offset) {
197+
return Err(ErrorCode::InvalidTimezone(
198+
"Timezone offset minute must be in [00, 59]",
199+
));
206200
}
207-
208201
let seconds = hour_offset * 3600 + minute_offset * 60;
209-
Ok(if west_tz { -seconds } else { seconds })
202+
let offset = if west_tz { -seconds } else { seconds };
203+
check_timezone_offset(offset)?;
204+
Ok(offset)
210205
}
211206

212207
let n = cursor.keep_read(buf, |f| f.is_ascii_digit());
@@ -216,12 +211,6 @@ fn read_offset_seconds<T: AsRef<[u8]>>(
216211
.map_err_to_code(ErrorCode::BadBytes, || {
217212
"hour offset parse error".to_string()
218213
})?;
219-
if !(0..=MAX_OFFSET_HOURS).contains(&hour_offset) {
220-
return Err(ErrorCode::BadBytes(format!(
221-
"Invalid Timezone Offset: The hour offset '{}' is outside the valid range. Expected range is [00-14] within a timezone gap of [-14:00, +14:00]",
222-
hour_offset
223-
)));
224-
}
225214

226215
buf.clear();
227216
if !cursor.ignore_byte(b':') {
@@ -250,12 +239,6 @@ fn read_offset_seconds<T: AsRef<[u8]>>(
250239
})?;
251240
buf.clear();
252241

253-
if !(0..=MAX_OFFSET_HOURS).contains(&hour_offset) {
254-
return Err(ErrorCode::BadBytes(format!(
255-
"Invalid Timezone Offset: The hour offset '{}' is outside the valid range. Expected range is [00-14] within a timezone gap of [-14:00, +14:00]",
256-
hour_offset
257-
)));
258-
}
259242
validated(hour_offset, minute_offset, west_tz)
260243
}
261244
_ => Err(ErrorCode::BadBytes(

src/common/io/src/datetime.rs

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ pub fn parse_four_digits(bytes: &[u8]) -> Option<i32> {
5959
}
6060

6161
/// Calendar text and explicit date-part constructors keep the four-digit year
62-
/// contract. The wider SQL value range is headroom for arithmetic and timezone
63-
/// conversion, not an extension of accepted calendar input.
62+
/// contract. Intermediate arithmetic and timezone conversion may use wider
63+
/// calendar years, with the final result checked against the SQL type bounds.
6464
pub fn check_input_year(year: i32) -> Result<()> {
6565
if (1..=9999).contains(&year) {
6666
Ok(())
@@ -71,6 +71,20 @@ pub fn check_input_year(year: i32) -> Result<()> {
7171
}
7272
}
7373

74+
/// Validate an explicit numeric timezone offset in seconds.
75+
/// This input restriction does not apply to offsets resolved from named timezones.
76+
#[inline]
77+
pub fn check_timezone_offset(offset_seconds: i32) -> Result<()> {
78+
const MAX_OFFSET: i32 = 14 * 3600;
79+
if (-MAX_OFFSET..=MAX_OFFSET).contains(&offset_seconds) {
80+
Ok(())
81+
} else {
82+
Err(ErrorCode::InvalidTimezone(format!(
83+
"Timezone offset {offset_seconds} seconds is out of range [-14:00, +14:00]"
84+
)))
85+
}
86+
}
87+
7488
/// Parse ISO-8601-like timestamps: `YYYY-MM-DD HH:MM:SS[.ffffff][Z|(+|-)hh[:mm]]`.
7589
/// Returning `None` indicates that the input is not in the supported format.
7690
#[inline(always)]
@@ -188,16 +202,16 @@ pub fn parse_standard_timestamp(input: &[u8]) -> Option<Result<ParsedTimestamp>>
188202
idx += 2;
189203
}
190204

191-
if hour_offset > 14
192-
|| minute_offset >= 60
193-
|| (hour_offset == 14 && minute_offset != 0)
194-
{
195-
return Some(Err(ErrorCode::BadBytes(
196-
"Timezone offset out of range".to_string(),
205+
if minute_offset >= 60 {
206+
return Some(Err(ErrorCode::InvalidTimezone(
207+
"Timezone offset minute must be in [00, 59]",
197208
)));
198209
}
199-
200-
provided_offset = Some(sign * (hour_offset * 3600 + minute_offset * 60));
210+
let offset = sign * (hour_offset * 3600 + minute_offset * 60);
211+
if let Err(err) = check_timezone_offset(offset) {
212+
return Some(Err(err));
213+
}
214+
provided_offset = Some(offset);
201215
}
202216
_ => return None,
203217
}

src/query/datavalues/src/types/type_date.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ use super::type_id::TypeID;
1717
use crate::prelude::*;
1818

1919
// SQL DATE bounds, as days since 1970-01-01. Keep in sync with common-expression.
20-
// Out-of-range SQL values are errors, not silently replaced with another date.
21-
pub const DATE_MAX: i32 = 3_298_504; // 11000-12-31
20+
// Conversion and arithmetic paths retain their own overflow policies.
21+
pub const DATE_MAX: i32 = 2_932_896; // 9999-12-31
2222
pub const DATE_MIN: i32 = -719_162; // 0001-01-01
2323

2424
#[derive(Default, Clone, Hash, serde::Deserialize, serde::Serialize)]

src/query/expression/src/types/date.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,12 @@ use crate::values::Column;
3737
use crate::values::Scalar;
3838

3939
pub const DATE_FORMAT: &str = "%Y-%m-%d";
40-
/// Internal SQL DATE bounds, represented as days since 1970-01-01.
41-
/// Years through 11000 provide headroom for arithmetic and timezone conversion;
42-
/// calendar text and explicit date-part constructors still accept 0001..=9999.
43-
/// This keeps the UInt16 year extraction API. Computed extended dates can be
44-
/// displayed, but their text is not necessarily accepted as calendar input.
40+
/// SQL DATE bounds, represented as days since 1970-01-01.
41+
/// Calendar inputs and computed DATE values both use years 0001..=9999.
4542
/// 0001-01-01
4643
pub const DATE_MIN: i32 = -719_162;
47-
/// 11000-12-31
48-
pub const DATE_MAX: i32 = 3_298_504;
44+
/// 9999-12-31
45+
pub const DATE_MAX: i32 = 2_932_896;
4946

5047
/// Converts internal epoch days. SQL inputs must pass `check_date` first.
5148
pub fn date_from_days(days: impl AsPrimitive<i64>) -> NaiveDate {
@@ -71,7 +68,7 @@ pub fn check_date(days: i64) -> Result<i32, String> {
7168
if (i64::from(DATE_MIN)..=i64::from(DATE_MAX)).contains(&days) {
7269
Ok(days as i32)
7370
} else {
74-
Err("Invalid date: date is out of range [0001-01-01, 11000-12-31]".to_string())
71+
Err("Invalid date: date is out of range [0001-01-01, 9999-12-31]".to_string())
7572
}
7673
}
7774

src/query/expression/src/types/timestamp.rs

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ use databend_common_exception::ErrorCode;
2424
use databend_common_io::cursor_ext::BufferReadDateTimeExt;
2525
use databend_common_io::cursor_ext::DateTimeResType;
2626
use databend_common_io::cursor_ext::ReadBytesExt;
27+
pub use databend_common_io::datetime::check_timezone_offset;
2728
use num_traits::AsPrimitive;
2829

2930
use super::ArgType;
@@ -39,23 +40,21 @@ use crate::values::Scalar;
3940

4041
pub const TIMESTAMP_FORMAT: &str = "%Y-%m-%d %H:%M:%S%.6f";
4142
/// SQL TIMESTAMP and TIMESTAMP_TZ bounds, in UTC microseconds since 1970-01-01.
42-
/// Internal range for computed UTC instants: 0001..=11000. Calendar text and
43-
/// explicit date-part constructors retain 0001..=9999; chrono's wider range is
44-
/// not exposed as an expanded input contract.
45-
/// Validate the final UTC instant after timezone resolution: a valid instant may
46-
/// display in local year 0 or 11001. Converting that local date to SQL DATE must
47-
/// separately validate DATE_MIN/MAX. Conversion and arithmetic paths retain
48-
/// their legacy overflow policies; display conversion clamps to these bounds.
43+
/// Validate the final UTC instant after timezone resolution, not intermediate
44+
/// calendar fields. A valid instant may display in local year 0 or 10000.
45+
/// Converting that local date to SQL DATE must separately validate DATE_MIN/MAX.
46+
/// INTERVAL arithmetic reports out-of-range results; other paths retain their
47+
/// existing overflow policies. Display conversion clamps to these bounds.
4948
/// 0001-01-01 00:00:00.000000 UTC
5049
pub const TIMESTAMP_MIN: i64 = -62_135_596_800_000_000;
51-
/// 11000-12-31 23:59:59.999999 UTC
52-
pub const TIMESTAMP_MAX: i64 = 284_990_831_999_999_999;
50+
/// 9999-12-31 23:59:59.999999 UTC
51+
pub const TIMESTAMP_MAX: i64 = 253_402_300_799_999_999;
5352

5453
pub const MICROS_PER_SEC: i64 = 1_000_000;
5554
pub const MICROS_PER_MILLI: i64 = 1_000;
5655

5756
/// Clamp to the SQL UTC bounds before converting for display.
58-
/// Chrono has room for local year 0/11001 at these boundaries.
57+
/// Chrono has room for local year 0/10000 at these boundaries.
5958
pub fn timestamp_from_micros(micros: impl AsPrimitive<i64>, tz: &Tz) -> DateTime<Tz> {
6059
let micros = micros.as_().clamp(TIMESTAMP_MIN, TIMESTAMP_MAX);
6160
let seconds = micros.div_euclid(MICROS_PER_SEC);
@@ -83,7 +82,7 @@ pub fn check_timestamp(micros: i64) -> Result<i64, String> {
8382
if (TIMESTAMP_MIN..=TIMESTAMP_MAX).contains(&micros) {
8483
Ok(micros)
8584
} else {
86-
Err("Invalid date: timestamp is out of range [0001-01-01, 11000-12-31] UTC".to_string())
85+
Err("Invalid date: timestamp is out of range [0001-01-01, 9999-12-31] UTC".to_string())
8786
}
8887
}
8988

@@ -214,7 +213,7 @@ pub fn string_to_timestamp(
214213
}
215214
Ok(DateTimeResType::Date(_)) => Err(ErrorCode::BadArguments("unexpected argument")),
216215
Err(e) => match e.code() {
217-
ErrorCode::BAD_BYTES => Err(e),
216+
ErrorCode::BAD_BYTES | ErrorCode::INVALID_TIMEZONE => Err(e),
218217
_ => Err(ErrorCode::BadArguments("unexpected argument")),
219218
},
220219
}
@@ -225,8 +224,7 @@ pub fn timestamp_to_string(ts: i64, tz: &Tz) -> impl Display {
225224
timestamp_from_micros(ts, tz).format(TIMESTAMP_FORMAT)
226225
}
227226

228-
/// Render a microsecond-precision UTC timestamp. Years through 9999 use RFC 3339;
229-
/// extended years use ISO 8601's signed form (for example `+11000`).
227+
/// Render a microsecond-precision UTC timestamp in RFC 3339 format.
230228
#[inline]
231229
pub fn timestamp_to_rfc3339_utc(ts: i64) -> String {
232230
timestamp_from_micros(ts, &Tz::UTC)

src/query/expression/src/types/timestamp_tz.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ use databend_common_column::types::timestamp_tz;
2424
use databend_common_exception::ErrorCode;
2525
use databend_common_exception::Result;
2626
use databend_common_io::datetime::check_input_year;
27+
use databend_common_io::datetime::check_timezone_offset;
2728
use databend_common_io::datetime::parse_standard_timestamp as parse_iso_timestamp;
2829
use databend_common_timezone::LocalTimeResolution;
2930
use databend_common_timezone::resolve_local_datetime;
@@ -270,6 +271,7 @@ pub fn string_to_timestamp_tz<'a, F: FnOnce() -> &'a Tz>(
270271
for format in PARSE_FORMATS_WITH_OFFSET {
271272
if let Ok(value) = DateTime::parse_from_str(text, format) {
272273
check_input_year(value.year())?;
274+
check_timezone_offset(value.offset().local_minus_utc())?;
273275
let micros = i128::from(value.timestamp()) * 1_000_000
274276
+ i128::from(value.timestamp_subsec_micros());
275277
return build_timestamp_tz(micros, value.offset().local_minus_utc());

src/query/expression/src/utils/auto_detect_datetime.rs

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use chrono_tz::Tz;
2020
use databend_common_column::types::timestamp_tz;
2121
use databend_common_exception::ErrorCode;
2222
use databend_common_io::datetime::check_input_year;
23+
use databend_common_io::datetime::check_timezone_offset;
2324
use databend_common_timezone::fast_utc_from_local;
2425
use databend_common_timezone::offset_seconds_at;
2526

@@ -128,14 +129,22 @@ impl ParsedDateTime {
128129
}
129130
}
130131

131-
fn try_parse_formats(val: &str, tz: &Tz, formats: &[&str]) -> Option<(i64, i32)> {
132+
// A matched format with an invalid explicit offset is an error, not a signal
133+
// to try a more permissive parser. UTC range clamping remains the caller's policy.
134+
#[allow(clippy::result_large_err)]
135+
fn try_parse_formats(
136+
val: &str,
137+
tz: &Tz,
138+
formats: &[&str],
139+
) -> Result<Option<(i64, i32)>, ErrorCode> {
132140
for format in formats {
133141
let Some(parsed) = ParsedDateTime::parse(format, val) else {
134142
continue;
135143
};
136144

137145
match parsed.offset_seconds {
138146
Some(offset) => {
147+
check_timezone_offset(offset)?;
139148
let Some(date) = parsed.naive_date() else {
140149
continue;
141150
};
@@ -148,18 +157,20 @@ fn try_parse_formats(val: &str, tz: &Tz, formats: &[&str]) -> Option<(i64, i32)>
148157
};
149158
let micros = local.and_utc().timestamp() * MICROS_PER_SEC + parsed.micro as i64
150159
- offset as i64 * MICROS_PER_SEC;
151-
return Some((micros, offset));
160+
return Ok(Some((micros, offset)));
152161
}
153162
None => {
154163
let Some(micros) = fast_timestamp_from_parsed(&parsed, tz) else {
155164
continue;
156165
};
157-
let offset = offset_seconds_at(tz, micros.div_euclid(MICROS_PER_SEC))?;
158-
return Some((micros, offset));
166+
let Some(offset) = offset_seconds_at(tz, micros.div_euclid(MICROS_PER_SEC)) else {
167+
continue;
168+
};
169+
return Ok(Some((micros, offset)));
159170
}
160171
}
161172
}
162-
None
173+
Ok(None)
163174
}
164175

165176
pub fn fast_timestamp_from_parsed(parsed: &ParsedDateTime, tz: &Tz) -> Option<i64> {
@@ -175,10 +186,13 @@ pub fn fast_timestamp_from_parsed(parsed: &ParsedDateTime, tz: &Tz) -> Option<i6
175186
)
176187
}
177188

178-
pub fn auto_detect_timestamp(val: &str, tz: &Tz) -> Option<i64> {
179-
let (mut micros, _) = try_parse_formats(val, tz, AUTO_TS_FORMATS)?;
189+
#[allow(clippy::result_large_err)]
190+
pub fn auto_detect_timestamp(val: &str, tz: &Tz) -> Result<Option<i64>, ErrorCode> {
191+
let Some((mut micros, _)) = try_parse_formats(val, tz, AUTO_TS_FORMATS)? else {
192+
return Ok(None);
193+
};
180194
clamp_timestamp(&mut micros);
181-
Some(micros)
195+
Ok(Some(micros))
182196
}
183197

184198
pub fn auto_detect_date(val: &str) -> Option<i32> {
@@ -194,10 +208,13 @@ pub fn auto_detect_date(val: &str) -> Option<i32> {
194208
None
195209
}
196210

197-
pub fn auto_detect_timestamp_tz(val: &str, tz: &Tz) -> Option<timestamp_tz> {
198-
let (mut micros, offset) = try_parse_formats(val, tz, AUTO_TS_FORMATS)?;
211+
#[allow(clippy::result_large_err)]
212+
pub fn auto_detect_timestamp_tz(val: &str, tz: &Tz) -> Result<Option<timestamp_tz>, ErrorCode> {
213+
let Some((mut micros, offset)) = try_parse_formats(val, tz, AUTO_TS_FORMATS)? else {
214+
return Ok(None);
215+
};
199216
clamp_timestamp(&mut micros);
200-
Some(timestamp_tz::new(micros, offset))
217+
Ok(Some(timestamp_tz::new(micros, offset)))
201218
}
202219

203220
/// Parse a date string with optional auto-detect fallback.
@@ -226,12 +243,13 @@ pub fn parse_date_with_auto(val: &str, tz: &Tz, enable_auto: bool) -> Result<i32
226243
pub fn parse_timestamp_with_auto(val: &str, tz: &Tz, enable_auto: bool) -> Result<i64, ErrorCode> {
227244
match string_to_timestamp(val, tz) {
228245
Ok(micros) => Ok(micros),
246+
Err(e) if e.code() == ErrorCode::INVALID_TIMEZONE => Err(e),
229247
Err(e) => {
230248
if enable_auto {
231249
if let Some(micros) = parse_epoch_str(val) {
232250
return Ok(micros);
233251
}
234-
if let Some(micros) = auto_detect_timestamp(val, tz) {
252+
if let Some(micros) = auto_detect_timestamp(val, tz)? {
235253
return Ok(micros);
236254
}
237255
}
@@ -250,14 +268,15 @@ pub fn parse_timestamp_tz_with_auto(
250268
) -> Result<timestamp_tz, ErrorCode> {
251269
match string_to_timestamp_tz(val.as_bytes(), || tz) {
252270
Ok(ts_tz) => Ok(ts_tz),
271+
Err(e) if e.code() == ErrorCode::INVALID_TIMEZONE => Err(e),
253272
Err(e) => {
254273
if enable_auto {
255274
if let Some(micros) = parse_epoch_str(val) {
256275
let offset = offset_seconds_at(tz, micros.div_euclid(MICROS_PER_SEC))
257276
.expect("validated Databend timestamp has a timezone offset");
258277
return Ok(timestamp_tz::new(micros, offset));
259278
}
260-
if let Some(ts_tz) = auto_detect_timestamp_tz(val, tz) {
279+
if let Some(ts_tz) = auto_detect_timestamp_tz(val, tz)? {
261280
return Ok(ts_tz);
262281
}
263282
}

src/query/expression/tests/it/types.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,6 @@ fn test_timestamp_display_clamps_bounds() {
137137
(-1_000_001, -1_000_001),
138138
(-1, -1),
139139
(0, 0),
140-
(253_402_300_799_999_999, 253_402_300_799_999_999),
141140
(TIMESTAMP_MAX, TIMESTAMP_MAX),
142141
(TIMESTAMP_MAX + 1, TIMESTAMP_MAX),
143142
(i64::MAX, TIMESTAMP_MAX),
@@ -156,7 +155,7 @@ fn test_timestamp_display_clamps_bounds() {
156155
);
157156
assert_eq!(
158157
timestamp_to_rfc3339_utc(i64::MAX),
159-
"+11000-12-31T23:59:59.999999Z"
158+
"9999-12-31T23:59:59.999999Z"
160159
);
161160
assert_eq!(
162161
timestamp_to_rfc3339_utc(i64::MIN),

0 commit comments

Comments
 (0)