Fix parsing failure when having multiple ttl expressions #234
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -645,15 +645,25 @@ func (p *Parser) parseAlterTableModify(pos Pos) (AlterTableClause, error) { | |||||||||||
| case p.matchKeyword(KeywordColumn): | ||||||||||||
| return p.parseAlterTableModifyColumn(pos) | ||||||||||||
| case p.matchKeyword(KeywordTtl): | ||||||||||||
| ttlPos := p.Pos() | ||||||||||||
| _ = p.lexer.consumeToken() | ||||||||||||
| items, err := p.parseTTLClause(ttlPos, true) | ||||||||||||
| if err != nil { | ||||||||||||
|
Comment on lines
647
to
651
There was a problem hiding this comment.
With Useful? React with 👍 / 👎. |
||||||||||||
| return nil, err | ||||||||||||
| } | ||||||||||||
| listEnd := ttlPos | ||||||||||||
| if len(items) > 0 { | ||||||||||||
| listEnd = items[len(items)-1].End() | ||||||||||||
| } | ||||||||||||
|
Comment on lines
+654
to
+657
There was a problem hiding this comment.
Suggested change
|
||||||||||||
| ttlClause := &TTLClause{ | ||||||||||||
| TTLPos: ttlPos, | ||||||||||||
| ListEnd: listEnd, | ||||||||||||
| Items: items, | ||||||||||||
| } | ||||||||||||
| return &AlterTableModifyTTL{ | ||||||||||||
| ModifyPos: pos, | ||||||||||||
| StatementEnd: ttlClause.End(), | ||||||||||||
| TTL: ttlClause, | ||||||||||||
|
Comment on lines
+654
to
+666
There was a problem hiding this comment.
|
||||||||||||
| }, nil | ||||||||||||
| case p.matchKeyword(KeywordQuery): | ||||||||||||
| _ = p.lexer.consumeToken() | ||||||||||||
|
|
||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| ALTER TABLE db.t0 ON CLUSTER default_cluster | ||
| MODIFY TTL | ||
| toDateTime(timestamp / 1000000000) + INTERVAL 30 DAY TO DISK 'gcs', | ||
| toDateTime(timestamp / 1000000000) + INTERVAL 60 DAY; | ||
|
Comment on lines
+2
to
+4
There was a problem hiding this comment. This new test covers multiple TTL expressions, but it doesn’t cover the case where the last TTL expression has a TTL policy (e.g., |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| -- Origin SQL: | ||
| ALTER TABLE db.t0 ON CLUSTER default_cluster | ||
| MODIFY TTL | ||
| toDateTime(timestamp / 1000000000) + INTERVAL 30 DAY TO DISK 'gcs', | ||
| toDateTime(timestamp / 1000000000) + INTERVAL 60 DAY; | ||
|
|
||
|
|
||
| -- Format SQL: | ||
| ALTER TABLE db.t0 ON CLUSTER default_cluster MODIFY TTL toDateTime(timestamp / 1000000000) + INTERVAL 30 DAY TO DISK 'gcs', toDateTime(timestamp / 1000000000) + INTERVAL 60 DAY; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,182 @@ | ||
| [ | ||
| { | ||
| "AlterPos": 0, | ||
| "StatementEnd": 184, | ||
| "TableIdentifier": { | ||
| "Database": { | ||
| "Name": "db", | ||
| "QuoteType": 1, | ||
| "NamePos": 12, | ||
| "NameEnd": 14 | ||
| }, | ||
| "Table": { | ||
| "Name": "t0", | ||
| "QuoteType": 1, | ||
| "NamePos": 15, | ||
| "NameEnd": 17 | ||
| } | ||
| }, | ||
| "OnCluster": { | ||
| "OnPos": 18, | ||
| "Expr": { | ||
| "Name": "default_cluster", | ||
| "QuoteType": 1, | ||
| "NamePos": 29, | ||
| "NameEnd": 44 | ||
| } | ||
| }, | ||
| "AlterExprs": [ | ||
| { | ||
| "ModifyPos": 45, | ||
| "StatementEnd": 184, | ||
| "TTL": { | ||
| "TTLPos": 52, | ||
| "ListEnd": 184, | ||
| "Items": [ | ||
| { | ||
| "TTLPos": 52, | ||
| "Expr": { | ||
| "LeftExpr": { | ||
| "Name": { | ||
| "Name": "toDateTime", | ||
| "QuoteType": 1, | ||
| "NamePos": 60, | ||
| "NameEnd": 70 | ||
| }, | ||
| "Params": { | ||
| "LeftParenPos": 70, | ||
| "RightParenPos": 93, | ||
| "Items": { | ||
| "ListPos": 71, | ||
| "ListEnd": 93, | ||
| "HasDistinct": false, | ||
| "Items": [ | ||
| { | ||
| "Expr": { | ||
| "LeftExpr": { | ||
| "Name": "timestamp", | ||
| "QuoteType": 1, | ||
| "NamePos": 71, | ||
| "NameEnd": 80 | ||
| }, | ||
| "Operation": "/", | ||
| "RightExpr": { | ||
| "NumPos": 83, | ||
| "NumEnd": 93, | ||
| "Literal": "1000000000", | ||
| "Base": 10 | ||
| }, | ||
| "HasGlobal": false, | ||
| "HasNot": false | ||
| }, | ||
| "Alias": null | ||
| } | ||
| ] | ||
| }, | ||
| "ColumnArgList": null | ||
| } | ||
| }, | ||
| "Operation": "+", | ||
| "RightExpr": { | ||
| "IntervalPos": 97, | ||
| "Expr": { | ||
| "NumPos": 106, | ||
| "NumEnd": 108, | ||
| "Literal": "30", | ||
| "Base": 10 | ||
| }, | ||
| "Unit": { | ||
| "Name": "DAY", | ||
| "QuoteType": 1, | ||
| "NamePos": 109, | ||
| "NameEnd": 112 | ||
| } | ||
| }, | ||
| "HasGlobal": false, | ||
| "HasNot": false | ||
| }, | ||
| "Policy": { | ||
| "Item": { | ||
| "RulePos": 113, | ||
| "ToVolume": null, | ||
| "ToDisk": { | ||
| "LiteralPos": 122, | ||
| "LiteralEnd": 125, | ||
| "Literal": "gcs" | ||
| }, | ||
| "Action": null | ||
| }, | ||
| "Where": null, | ||
| "GroupBy": null | ||
| } | ||
| }, | ||
| { | ||
| "TTLPos": 52, | ||
| "Expr": { | ||
| "LeftExpr": { | ||
| "Name": { | ||
| "Name": "toDateTime", | ||
| "QuoteType": 1, | ||
| "NamePos": 132, | ||
| "NameEnd": 142 | ||
| }, | ||
| "Params": { | ||
| "LeftParenPos": 142, | ||
| "RightParenPos": 165, | ||
| "Items": { | ||
| "ListPos": 143, | ||
| "ListEnd": 165, | ||
| "HasDistinct": false, | ||
| "Items": [ | ||
| { | ||
| "Expr": { | ||
| "LeftExpr": { | ||
| "Name": "timestamp", | ||
| "QuoteType": 1, | ||
| "NamePos": 143, | ||
| "NameEnd": 152 | ||
| }, | ||
| "Operation": "/", | ||
| "RightExpr": { | ||
| "NumPos": 155, | ||
| "NumEnd": 165, | ||
| "Literal": "1000000000", | ||
| "Base": 10 | ||
| }, | ||
| "HasGlobal": false, | ||
| "HasNot": false | ||
| }, | ||
| "Alias": null | ||
| } | ||
| ] | ||
| }, | ||
| "ColumnArgList": null | ||
| } | ||
| }, | ||
| "Operation": "+", | ||
| "RightExpr": { | ||
| "IntervalPos": 169, | ||
| "Expr": { | ||
| "NumPos": 178, | ||
| "NumEnd": 180, | ||
| "Literal": "60", | ||
| "Base": 10 | ||
| }, | ||
| "Unit": { | ||
| "Name": "DAY", | ||
| "QuoteType": 1, | ||
| "NamePos": 181, | ||
| "NameEnd": 184 | ||
| } | ||
| }, | ||
| "HasGlobal": false, | ||
| "HasNot": false | ||
| }, | ||
| "Policy": null | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AlterTableModifyTTL.TTLchanged from*TTLExprto*TTLClause, which is a breaking change for any external consumers of this package’s AST. If this repo aims to keep API stability, consider preserving the old field (e.g., add a newTTLClausefield while keepingTTLExprfor the single-item case), or provide a helper accessor/migration path and document the breaking change (potentially requiring a major version bump).