hililiwei commented on code in PR #6637:
URL: https://github.com/apache/iceberg/pull/6637#discussion_r1083582974
##########
spark/v3.3/spark-extensions/src/main/antlr/org.apache.spark.sql.catalyst.parser.extensions/IcebergSqlExtensions.g4:
##########
@@ -168,34 +169,61 @@ fieldList
;
nonReserved
- : ADD | ALTER | AS | ASC | BY | CALL | DESC | DROP | FIELD | FIRST | LAST
| NULLS | ORDERED | PARTITION | TABLE | WRITE
- | DISTRIBUTED | LOCALLY | UNORDERED | REPLACE | WITH | IDENTIFIER_KW |
FIELDS | SET
- | TRUE | FALSE
+ : ADD | ALTER | AS | ASC | BY | CALL | CREATE | DAYS | DESC | DROP | FIELD
| FIRST | HOURS | LAST | NULLS | OF | ORDERED | PARTITION | TABLE | WRITE
+ | DISTRIBUTED | LOCALLY | MINUTES | UNORDERED | REPLACE | VERSION | WITH |
IDENTIFIER_KW | FIELDS | SET
+ | TAG | TRUE | FALSE
| MAP
;
+snapshotId
+ : number
+ ;
+
+snapshotRefRetain
Review Comment:
>To me, it feels more intuitive to just say RETAIN number timeUnit instead
of RETAIN snapshotRefRetain snapshotRefRetainTimeUnit
I originally added `snapshotRefRetain` and `snapshotRetain` to make the
statement parsing code more readable. Removing it is technically feasible. In
the new version, I have removed (including create branch).
>Another thing I overlooked is that looks like the Antlr convention is to
use all capital letters for these definitions, like TIME_UNIT instead of
timeUnit. I don't know if they imply different functionalities though.
All caps usually indicate keywords. So, I prefer keep `timeUnit`.
```
ALTER TABLE multipartIdentifier WRITE writeSpec
#setWriteDistributionAndOrdering
```
like `TABLE`:
`TABLE: 'TABLE';`
However, `writeSpec` is written as:
```
writeSpec
: (writeDistributionSpec | writeOrderingSpec)*
;
```
`writeDistributionSpec`:
```
writeDistributionSpec
: DISTRIBUTED BY PARTITION
;
```
Thanks.
Liwei
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
