Skip to content

Commit

Permalink
parser: Fix regression in parsing of timestamp as identifier (#22362)
Browse files Browse the repository at this point in the history
parser: Fix regression in parsing of timestamp as identifier
  • Loading branch information
Mouli Mukherjee authored Oct 17, 2023
1 parent ea7b33c commit 3ff5a28
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 53 deletions.
44 changes: 23 additions & 21 deletions src/sql-parser/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,9 @@ impl<'a> Parser<'a> {
// because the `date` type name is not followed by a string literal, but
// in fact is a valid expression that should parse as the column name
// "date".
//
// Note: the maybe! block here does swallow valid parsing errors
// See <https://github.com/MaterializeInc/materialize/issues/22397> for more details
maybe!(self.maybe_parse(|parser| {
let data_type = parser.parse_data_type()?;
if data_type.to_string().as_str() == "interval" {
Expand Down Expand Up @@ -565,15 +568,6 @@ impl<'a> Parser<'a> {
Token::Keyword(EXISTS) => self.parse_exists_expr(),
Token::Keyword(EXTRACT) => self.parse_extract_expr(),
Token::Keyword(INTERVAL) => Ok(Expr::Value(self.parse_interval_value()?)),
// having timestamps separately here because errors are swallowed by the maybe! block
Token::Keyword(TIMESTAMP) | Token::Keyword(TIMESTAMPTZ) => {
self.prev_token();
let data_type = self.parse_data_type()?;
Ok(Expr::Cast {
expr: Box::new(Expr::Value(Value::String(self.parse_literal_string()?))),
data_type,
})
}
Token::Keyword(NOT) => Ok(Expr::Not {
expr: Box::new(self.parse_subexpr(Precedence::PrefixNot)?),
}),
Expand Down Expand Up @@ -5226,13 +5220,26 @@ impl<'a> Parser<'a> {

/// Parse a signed literal integer.
fn parse_literal_int(&mut self) -> Result<i64, ParserError> {
let negative = self.consume_token(&Token::Op("-".into()));
match self.next_token() {
Some(Token::Number(s)) => s.parse::<i64>().map_err(|e| {
self.error(
self.peek_prev_pos(),
format!("Could not parse '{}' as i64: {}", s, e),
)
}),
Some(Token::Number(s)) => {
let n = s.parse::<i64>().map_err(|e| {
self.error(
self.peek_prev_pos(),
format!("Could not parse '{}' as i64: {}", s, e),
)
})?;
if negative {
n.checked_neg().ok_or_else(|| {
self.error(
self.peek_prev_pos(),
format!("Could not parse '{}' as i64: overflows i64", s),
)
})
} else {
Ok(n)
}
}
other => self.expected(self.peek_prev_pos(), "literal integer", other),
}
}
Expand Down Expand Up @@ -5413,12 +5420,7 @@ impl<'a> Parser<'a> {
// parses the precision in timestamp(<precision>) and timestamptz(<precision>)
fn parse_timestamp_precision(&mut self) -> Result<Vec<i64>, ParserError> {
if self.consume_token(&Token::LParen) {
let typ_mod = self.parse_literal_uint()?.try_into().map_err(|_| {
self.error(
self.index,
"number too large to fit in target type".to_string(),
)
})?;
let typ_mod = self.parse_literal_int()?;
self.expect_token(&Token::RParen)?;
Ok(vec![typ_mod])
} else {
Expand Down
32 changes: 12 additions & 20 deletions src/sql-parser/tests/testdata/literal
Original file line number Diff line number Diff line change
Expand Up @@ -173,30 +173,22 @@ Cast { expr: Value(String("invalid timestamptx")), data_type: Other { name: Name
parse-scalar
TIMESTAMP(-1) '1999-01-01 01:23:34.555'
----
error: Expected literal unsigned integer, found operator "-"
TIMESTAMP(-1) '1999-01-01 01:23:34.555'
^
Cast { expr: Value(String("1999-01-01 01:23:34.555")), data_type: Other { name: Name(UnresolvedItemName([Ident("timestamp")])), typ_mod: [-1] } }

parse-scalar
TIMESTAMPTZ(-1) '1999-01-01 01:23:34.555'
----
error: Expected literal unsigned integer, found operator "-"
TIMESTAMPTZ(-1) '1999-01-01 01:23:34.555'
^
Cast { expr: Value(String("1999-01-01 01:23:34.555")), data_type: Other { name: Name(UnresolvedItemName([Ident("timestamptz")])), typ_mod: [-1] } }

parse-scalar
TIMESTAMP(-1) WITH TIME ZONE '1999-01-01 01:23:34.555'
----
error: Expected literal unsigned integer, found operator "-"
TIMESTAMP(-1) WITH TIME ZONE '1999-01-01 01:23:34.555'
^
Cast { expr: Value(String("1999-01-01 01:23:34.555")), data_type: Other { name: Name(UnresolvedItemName([Ident("timestamptz")])), typ_mod: [-1] } }

parse-scalar
TIMESTAMP(-1) WITHOUT TIME ZONE '1999-01-01 01:23:34.555'
----
error: Expected literal unsigned integer, found operator "-"
TIMESTAMP(-1) WITHOUT TIME ZONE '1999-01-01 01:23:34.555'
^
Cast { expr: Value(String("1999-01-01 01:23:34.555")), data_type: Other { name: Name(UnresolvedItemName([Ident("timestamp")])), typ_mod: [-1] } }

parse-scalar
TIMESTAMP(3) '1970-01-01T00:00:00.666666666'
Expand Down Expand Up @@ -241,30 +233,30 @@ Cast { expr: Value(String("1970-01-01T00:00:00.666666666")), data_type: Other {
parse-scalar
TIMESTAMP(9223372036854775808) '1970-01-01T00:00:00.666666666'
----
error: number too large to fit in target type
error: extra token after expression
TIMESTAMP(9223372036854775808) '1970-01-01T00:00:00.666666666'
^
^

parse-scalar
TIMESTAMPTZ(9223372036854775808) '1970-01-01T00:00:00.666666666'
----
error: number too large to fit in target type
error: extra token after expression
TIMESTAMPTZ(9223372036854775808) '1970-01-01T00:00:00.666666666'
^
^

parse-scalar
TIMESTAMP(9223372036854775808) WITH TIME ZONE '1970-01-01T00:00:00.666666666'
----
error: number too large to fit in target type
error: extra token after expression
TIMESTAMP(9223372036854775808) WITH TIME ZONE '1970-01-01T00:00:00.666666666'
^
^

parse-scalar
TIMESTAMP(9223372036854775808) WITHOUT TIME ZONE '1970-01-01T00:00:00.666666666'
----
error: number too large to fit in target type
error: extra token after expression
TIMESTAMP(9223372036854775808) WITHOUT TIME ZONE '1970-01-01T00:00:00.666666666'
^
^

parse-scalar
INTERVAL '1' YEAR
Expand Down
4 changes: 1 addition & 3 deletions src/sql-parser/tests/testdata/scalar
Original file line number Diff line number Diff line change
Expand Up @@ -232,9 +232,7 @@ parse-scalar roundtrip
parse-scalar roundtrip
(id::timestamp(5) with time zone::timestamp(-1) without time zone ) :: double precision::text
----
error: Expected literal unsigned integer, found operator "-"
(id::timestamp(5) with time zone::timestamp(-1) without time zone ) :: double precision::text
^
(id::timestamptz(5)::timestamp(-1))::float8::text

parse-scalar roundtrip
CAST(c::jsonb->>'f' AS timestamptz)
Expand Down
39 changes: 31 additions & 8 deletions test/sqllogictest/dates-times.slt
Original file line number Diff line number Diff line change
Expand Up @@ -1370,7 +1370,7 @@ SELECT now() + null;
NULL

# Test timestamp precision.
query error Expected literal unsigned integer
query error precision for type timestamp or timestamptz must be between 0 and 6
SELECT '1970-01-01T00:00:00.666666'::timestamp(-1);

query error db error: ERROR: precision for type timestamp or timestamptz must be between 0 and 6
Expand Down Expand Up @@ -1562,25 +1562,27 @@ SELECT timestamp(5) '95143-12-31 17:59:59.1235+00' = timestamp(4) '95143-12-31 1
----
true

query error Expected literal unsigned integer
query error precision for type timestamp or timestamptz must be between 0 and 6
SELECT timestamp(-1) '95143-12-31 23:59:59.123456789+06';

query error Expected literal unsigned integer
query error precision for type timestamp or timestamptz must be between 0 and 6
SELECT timestamp(-1) with time zone '95143-12-31 23:59:59.123456789+06';

query error Expected literal unsigned integer
query error precision for type timestamp or timestamptz must be between 0 and 6
SELECT '95143-12-31 23:59:59.123456789+06'::timestamp(-1);

query error Expected literal unsigned integer
query error precision for type timestamp or timestamptz must be between 0 and 6
SELECT timestamptz(-1) '95143-12-31 23:59:59.123456789+06';

query error number too large to fit in target type
# Note: we regressed the error messages for the following
# See <https://github.com/MaterializeInc/materialize/issues/22397>
query error Expected end of statement
SELECT timestamp(10000000000000000000) '95143-12-31 23:59:59.123456789+06';

query error number too large to fit in target type
query error Expected end of statement
SELECT timestamptz(10000000000000000000) '95143-12-31 23:59:59.123456789+06';

query error number too large to fit in target type
query error Expected end of statement
SELECT timestamp(9223372036854775808) '95143-12-31 23:59:59.123456789+06';

# checking we are not converting between the same precision again
Expand Down Expand Up @@ -1613,3 +1615,24 @@ Map (adjust_timestamp_precision(text_to_timestamp("95143-12-31 23:59:59.12345678
- ()

EOF

statement ok
CREATE table t (timestamp string);

statement ok
INSERT INTO t VALUES('2012-03-05');

query T
SELECT * from t ORDER BY timestamp;
----
2012-03-05

query T
SELECT * from t ORDER BY t.timestamp;
----
2012-03-05

query T
SELECT timestamp::timestamp from t ORDER BY t.timestamp;
----
2012-03-05 00:00:00
2 changes: 1 addition & 1 deletion test/testdrive/dates-times.td
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ $ kafka-verify-data format=avro sink=materialize.public.ts_precision_sink sort-m
{"key": true} {"key": true, "ts": 123456, "ts0": 0, "ts1": 100, "ts2": 120, "ts3": 123, "ts4": 123500, "ts5": 123460, "ts6": 123456, "tstz": 123456, "tstz0": 0, "tstz1": 100, "tstz2": 120, "tstz3": 123, "tstz4": 123500, "tstz5": 123460, "tstz6": 123456}

! SELECT '1970-01-01T00:00:00.123456'::timestamp(-1)
contains:Expected literal unsigned integer
contains:precision for type timestamp or timestamptz must be between 0 and 6

! SELECT '1970-01-01T00:00:00.123456'::timestamp(7)
contains:precision for type timestamp or timestamptz must be between 0 and 6
Expand Down

0 comments on commit 3ff5a28

Please sign in to comment.