INTERVAL support, date pseudo literals TIMESTAMP and TIME literals. by kchasapis · Pull Request #255 · hyrise/sql-parser

kchasapis · 2025-06-02T21:47:55Z

The interval was not allowed to be a column data type. With this patch we add this support.

We add support for the pseudo literals
CURRENT_TIME, CURRENT_TIMESTAMP, CURRENT_DATE.

Also, we add support for TIMESTAMP and TIME literals. The user can now explicitly set the type of the string value it follows with TIMESTAMP 'value' and TIME 'value.

DATE was already supported.

dey4ss

Thank you for your contribution! Before I go into detail reviewing your changes, can you please add some unit tests?

dey4ss · 2025-06-12T08:09:09Z

src/sql/CreateStatement.h

  ColumnType type;
  bool nullable;
  std::vector<ReferencesSpecification*>* references;
+  Expr* defaultValue{nullptr};


This member is unused, isn't it?

@dey4ss I have deleted it.

The interval was not allowed to be a column data type. With this patch we add this support. We add support for the pseudo literals CURRENT_TIME, CURRENT_TIMESTAMP, CURRENT_DATE. Also, we add support for TIMESTAMP and TIME literals. The user can now explicitly set the type of the string value it follows with TIMESTAMP 'value' and TIME 'value. DATE was already supported.

kchasapis · 2025-06-16T22:45:16Z

Thank you for your contribution! Before I go into detail reviewing your changes, can you please add some unit tests?

Thanks for your message. I have added unit tests. Please, let me know if there is anything else needed from my side to get started in the review.

dey4ss

Sorry for the late review. We discussed the PR internally and we have a few suggestions.

dey4ss · 2025-10-30T09:31:09Z

src/sql/Expr.h

+  kExprCurrentTimestamp,
+  kExprCurrentDate,
+  kExprCurrentTime,


We discussed this internally and we prefer to have only an expression type per date/time/timestamp, not for the "current" version of the literal. Instead, please add a (boolean) flag to the Expr struct indicating whether the literal has the special "current" value.

Mini: The expression types are not really ordered by name. Could you bring them in the order date, time, timestamp, interval?

dey4ss · 2025-10-30T09:33:27Z

src/sql/Expr.h

+  static Expr* makeTimestampLiteral(char* val);
+
+  static Expr* makeTimeLiteral(char* val);
+
  static Expr* makeIntervalLiteral(int64_t duration, DatetimeField unit);

+  static Expr* makeCurrentTimestamp();
+
+  static Expr* makeCurrentDate();
+
+  static Expr* makeCurrentTime();


Please see my previous comment. The "current" versions of these functions are not required when we always pass a char*. If the pointer is a nullptr, we could automatically set the flag to mark the "current" literal version.

dey4ss · 2025-10-30T09:36:28Z

test/queries/queries-good.sql

@@ -1,4 +1,4 @@
-# This file contains a list of strings that are NOT valid SQL queries.
+# This file contains a list of strings that are valid SQL queries.


Nice catch!

dey4ss · 2025-10-30T10:19:25Z

src/parser/bison_parser.y

+| INTERVAL INTVAL datetime_field {
+  $$ = Expr::makeIntervalLiteral($2, $3);
+}


Do you have a use case for this version? According to the SQL:2023 standard, only

INTERVAL 'INTVAL' duration_field

would be valid (note the quotation marks). However, we supported some more versions according to Postgres. INTERVAL INTVAL datetime_field does not seem to work in Postgres (see DB Fiddle, statements in comments raise errors).

dey4ss · 2025-10-30T10:22:39Z

src/parser/bison_parser.y

+current_timestamp : CURRENT_TIMESTAMP { $$ = Expr::makeCurrentTimestamp(); };
+
+current_date : CURRENT_DATE { $$ = Expr::makeCurrentDate(); };
+
+current_time : CURRENT_TIME { $$ = Expr::makeCurrentTime(); };


Can you please merge these with the regular versions? E.g., merge current_timestamp with timestamp_literal to the following:

timestamp_literal : TIMESTAMP STRING { $$ = Expr::makeTimestampLiteral($2); } | CURRENT_TIMESTAMP { $$ = Expr::makeTimestampLiteral(nullptr); };

dey4ss · 2025-10-30T10:25:23Z

src/parser/flex_lexer.l

+CURRENT_TIMESTAMP        TOKEN(CURRENT_TIMESTAMP)
+CURRENT_DATE             TOKEN(CURRENT_DATE)
+CURRENT_TIME             TOKEN(CURRENT_TIME)


Can you please order lexicographically (date < time < timestamp)?

dey4ss · 2025-10-30T10:27:04Z

src/parser/bison_parser.y

 %type <expr>                   expr operand scalar_expr unary_expr binary_expr logic_expr exists_expr extract_expr cast_expr
 %type <expr>                   function_expr between_expr expr_alias param_expr
 %type <expr>                   column_name literal int_literal num_literal string_literal bool_literal date_literal interval_literal
+%type <expr>                   timestamp_literal time_literal current_timestamp current_date current_time


(see previous comment) Please remove the current_ versions here.

dey4ss · 2025-10-30T10:29:01Z

src/sql/Expr.cpp

+         isType(kExprLiteralNull) || isType(kExprLiteralDate) || isType(kExprLiteralInterval) ||
+         isType(kExprLiteralTimestamp) || isType(kExprLiteralTime);


The "current" versions are missing here. However, we would suggest that they don't have own types anyways.

dey4ss · 2025-10-30T10:31:35Z

test/sql_tests.cpp

+  SQLParser::parse(
+      "CREATE TABLE interval_table (c_interval INTERVAL )",
+      &result);


You can use TEST_PARSE_SINGLE_SQL here. This macro also asserts that the statement is valid and that it returns a single statement of the desired type and of the desired class.

dey4ss · 2025-10-30T10:38:24Z

test/sql_tests.cpp

+  TEST_PARSE_SINGLE_SQL("SELECT * FROM students WHERE graduation_date = CURRENT_DATE;",
+                       kStmtSelect, SelectStatement, result, stmt);
+
+  ASSERT_TRUE(result.isValid());


pick here: TEST_PARSE_SINGLE_SQL already asserts this.

kchasapis force-pushed the interval_date_psedo_literals branch from 08fe178 to d2da311 Compare June 4, 2025 14:52

dey4ss reviewed Jun 12, 2025

View reviewed changes

kchasapis force-pushed the interval_date_psedo_literals branch from d2da311 to 9109fe0 Compare June 16, 2025 22:36

kchasapis force-pushed the interval_date_psedo_literals branch from 9109fe0 to ef49bda Compare June 16, 2025 22:43

dey4ss reviewed Oct 30, 2025

View reviewed changes

		@@ -1,4 +1,4 @@
		# This file contains a list of strings that are NOT valid SQL queries.
		# This file contains a list of strings that are valid SQL queries.

		isType(kExprLiteralNull) \|\| isType(kExprLiteralDate) \|\| isType(kExprLiteralInterval) \|\|
		isType(kExprLiteralTimestamp) \|\| isType(kExprLiteralTime);

Conversation

kchasapis commented Jun 2, 2025

Uh oh!

dey4ss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kchasapis commented Jun 16, 2025

Uh oh!

dey4ss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants