Implement date, time and timestamp literals #10921

dbussink · 2022-08-03T12:41:19Z

This implements the date, time and timestamp literals in the parser and ensures that bind variables also can be generated using these literals.

Related Issue(s)

Part of #8604

Checklist

"Backport me!" label has been added if this change should be backported
Tests were added or are not required
Documentation was added or is not required

vitess-bot · 2022-08-03T12:41:22Z

dbussink · 2022-08-03T12:41:55Z

@GuptaManan100 This adds a number of shift / reduce conflicts and I was wondering if you think it's possible to fix those or not here.

dbussink · 2022-08-03T12:43:16Z

The other main question I have here is how to deal with values that aren't valid date, time or timestamps. What is the general approach we should take for those?

GuptaManan100

The shift-reduce conflicts have been fixed using precedence rules.
STRING_TYPE_PREFIX_NON_KEYWORD is used to resolve shift-reduce conflicts occurring due to column_name symbol and being able to use keywords like DATE and TIME as prefixes to strings to denote their type. The shift-reduce conflict occurs because after seeing one of these non-reserved keywords, if we see a STRING, then we can either shift to use the STRING typed rule in literal or reduce the non-reserved keyword into column_name and eventually use a rule from simple_expr. The way to fix this conflict is to give shifting higher precedence than reducing.

dbussink · 2022-08-06T18:35:43Z

@GuptaManan100 @systay I've also added logic here to validate the actual literals and tried to match MySQL behavior within reason.

dbussink · 2022-08-06T18:36:30Z

The CI failure here in endtoend is unrelated to the changes here, see also #10953

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>

… column names Signed-off-by: Manan Gupta <manan@planetscale.com>

…used to type cast STRINGS Signed-off-by: Manan Gupta <manan@planetscale.com>

Signed-off-by: Manan Gupta <manan@planetscale.com>

This adds additional validation of date, time and timestamp literals. It uses the same validation the evalengine already uses today which is more restricted than MySQL itself but seems good enough for now. It consolidates the parsing into one place so if the syntax allowed is extended, we only have to update one location. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>

dbussink · 2022-08-07T08:05:57Z

go/mysql/constants.go

@@ -541,6 +541,7 @@ const (
 	ERInvalidCastToJSON            = 3147
 	ERJSONValueTooBig              = 3150
 	ERJSONDocumentTooDeep          = 3157
+	ERWrongValue                   = 1525


This is the same error type as MySQL:

mysql> select date'foo'; ERROR 1525 (HY000): Incorrect DATE value: 'foo'

dbussink · 2022-08-07T08:06:23Z

go/mysql/sql_error.go

@@ -189,6 +189,7 @@ var stateToMysqlCode = map[vterrors.State]struct {
 	vterrors.WrongNumberOfColumnsInSelect: {num: ERWrongNumberOfColumnsInSelect, state: SSWrongNumberOfColumns},
 	vterrors.WrongTypeForVar:              {num: ERWrongTypeForVar, state: SSClientError},
 	vterrors.WrongValueForVar:             {num: ERWrongValueForVar, state: SSClientError},
+	vterrors.WrongValue:                   {num: ERWrongValue, state: SSUnknownSQLState},


The SQL state here is also in MySQL unknown (HY000):

mysql> select date'foo'; ERROR 1525 (HY000): Incorrect DATE value: 'foo'

dbussink · 2022-08-07T10:21:05Z

go/vt/sqlparser/normalizer.go

@@ -146,6 +163,11 @@ func (nz *normalizer) convertLiteralDedup(node *Literal, cursor *Cursor) {

 // convertLiteral converts an Literal without the dedup.
 func (nz *normalizer) convertLiteral(node *Literal, cursor *Cursor) {
+	err := validateLiteral(node)


Tried various approaches, but injecting this in the normalization step seemed the easiest but it needs to be checked then from here and also in convertLiteralDedup.

go/vt/sqlparser/parse_date.go

dbussink · 2022-08-07T10:23:28Z

go/vt/vtgate/evalengine/evalengine.go

@@ -235,39 +235,27 @@ func compareNumeric(v1, v2 *EvalResult) (int, error) {
 func parseDate(expr *EvalResult) (t time.Time, err error) {


Unified what this function depends on also into the single parsing logic in the sqlparser package.

dbussink · 2022-08-07T10:23:59Z

go/vt/vtgate/evalengine/evalengine.go

 	}
+	t, err = sqlparser.ParseTime(expr.string())
 	return


The new logic above should be equivalent to trying things in the loop. The error message depends on the last type checked, but that already was the case anyway.

GuptaManan100

I like this approach 🥇

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>

…o#929) * Implement date, time and timestamp literals Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * feat: add a test that verifies we can use date, timestamp and time as column names Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: add precedence rule for non-reserved-keywords that can also be used to type cast STRINGS Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix tpch test to not expect syntax error Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: make parser Signed-off-by: Manan Gupta <manan@planetscale.com> * Add validate of date style literals This adds additional validation of date, time and timestamp literals. It uses the same validation the evalengine already uses today which is more restricted than MySQL itself but seems good enough for now. It consolidates the parsing into one place so if the syntax allowed is extended, we only have to update one location. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Implement more complete TIME parsing support Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Co-authored-by: Manan Gupta <manan@planetscale.com> Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>

GuptaManan100 approved these changes Aug 3, 2022

View reviewed changes

dbussink added Type: Enhancement Logical improvement (somewhere between a bug and feature) Component: Query Serving labels Aug 3, 2022

dbussink force-pushed the parse-date-time-timestamp-literals branch from 32731aa to f6f7e20 Compare August 6, 2022 16:07

dbussink marked this pull request as ready for review August 6, 2022 18:35

dbussink requested review from harshit-gangal, systay, frouioui and mattlord as code owners August 6, 2022 18:35

dbussink and others added 5 commits August 6, 2022 21:56

Implement date, time and timestamp literals

393c508

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>

feat: add a test that verifies we can use date, timestamp and time as…

8822d1e

… column names Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: add precedence rule for non-reserved-keywords that can also be …

0e8f838

…used to type cast STRINGS Signed-off-by: Manan Gupta <manan@planetscale.com>

test: fix tpch test to not expect syntax error

d574c57

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: make parser

6913fd6

Signed-off-by: Manan Gupta <manan@planetscale.com>

dbussink force-pushed the parse-date-time-timestamp-literals branch from f6f7e20 to 5106666 Compare August 6, 2022 19:56

dbussink force-pushed the parse-date-time-timestamp-literals branch from 5106666 to a48a161 Compare August 7, 2022 08:08

dbussink commented Aug 7, 2022

View reviewed changes

GuptaManan100 approved these changes Aug 8, 2022

View reviewed changes

Implement more complete TIME parsing support

865d610

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>

dbussink force-pushed the parse-date-time-timestamp-literals branch from 57046c3 to 865d610 Compare August 8, 2022 08:08

GuptaManan100 merged commit 6450e49 into vitessio:main Aug 8, 2022

dbussink deleted the parse-date-time-timestamp-literals branch August 8, 2022 18:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement date, time and timestamp literals #10921

Implement date, time and timestamp literals #10921

dbussink commented Aug 3, 2022 •

edited

Loading

vitess-bot bot commented Aug 3, 2022 •

edited by GuptaManan100

Loading

dbussink commented Aug 3, 2022

dbussink commented Aug 3, 2022 •

edited

Loading

GuptaManan100 left a comment •

edited

Loading

dbussink commented Aug 6, 2022

dbussink commented Aug 6, 2022

dbussink Aug 7, 2022

dbussink Aug 7, 2022

dbussink Aug 7, 2022

dbussink Aug 7, 2022

dbussink Aug 7, 2022

GuptaManan100 left a comment

		@@ -235,39 +235,27 @@ func compareNumeric(v1, v2 *EvalResult) (int, error) {
		func parseDate(expr *EvalResult) (t time.Time, err error) {

Implement date, time and timestamp literals #10921

Implement date, time and timestamp literals #10921

Conversation

dbussink commented Aug 3, 2022 • edited Loading

Related Issue(s)

Checklist

vitess-bot bot commented Aug 3, 2022 • edited by GuptaManan100 Loading

Review Checklist

General

Bug fixes

Non-trivial changes

New/Existing features

Backward compatibility

dbussink commented Aug 3, 2022

dbussink commented Aug 3, 2022 • edited Loading

GuptaManan100 left a comment • edited Loading

Choose a reason for hiding this comment

dbussink commented Aug 6, 2022

dbussink commented Aug 6, 2022

dbussink Aug 7, 2022

Choose a reason for hiding this comment

dbussink Aug 7, 2022

Choose a reason for hiding this comment

dbussink Aug 7, 2022

Choose a reason for hiding this comment

dbussink Aug 7, 2022

Choose a reason for hiding this comment

dbussink Aug 7, 2022

Choose a reason for hiding this comment

GuptaManan100 left a comment

Choose a reason for hiding this comment

dbussink commented Aug 3, 2022 •

edited

Loading

vitess-bot bot commented Aug 3, 2022 •

edited by GuptaManan100

Loading

dbussink commented Aug 3, 2022 •

edited

Loading

GuptaManan100 left a comment •

edited

Loading