carlfu-db commented on code in PR #38404:
URL: https://github.com/apache/spark/pull/38404#discussion_r1013470376
##########
sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -319,7 +319,7 @@ query
insertInto
: INSERT OVERWRITE TABLE? multipartIdentifier (partitionSpec (IF NOT
EXISTS)?)? identifierList? #insertOverwriteTable
| INSERT INTO TABLE? multipartIdentifier partitionSpec? (IF NOT EXISTS)?
identifierList? #insertIntoTable
- | INSERT INTO TABLE? multipartIdentifier REPLACE whereClause?
identifierList? #insertIntoReplaceWhere
+ | INSERT INTO TABLE? multipartIdentifier REPLACE whereClause?
identifierList #insertIntoReplaceWhere
Review Comment:
This change can make the `SQLQueryTestSuite interval.sql` test success
again, but it won't work for the current PR. cc @cloud-fan
##########
sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -319,7 +319,7 @@ query
insertInto
: INSERT OVERWRITE TABLE? multipartIdentifier (partitionSpec (IF NOT
EXISTS)?)? identifierList? #insertOverwriteTable
| INSERT INTO TABLE? multipartIdentifier partitionSpec? (IF NOT EXISTS)?
identifierList? #insertIntoTable
- | INSERT INTO TABLE? multipartIdentifier REPLACE whereClause?
identifierList? #insertIntoReplaceWhere
+ | INSERT INTO TABLE? multipartIdentifier REPLACE whereClause?
identifierList #insertIntoReplaceWhere
Review Comment:
<img width="1419" alt="image"
src="https://user-images.githubusercontent.com/114777395/199851448-df273e7b-4a8b-4dc8-875f-b47882d8c265.png";>
##########
sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -319,6 +319,7 @@ query
insertInto
: INSERT OVERWRITE TABLE? multipartIdentifier (partitionSpec (IF NOT
EXISTS)?)? identifierList? #insertOverwriteTable
| INSERT INTO TABLE? multipartIdentifier partitionSpec? (IF NOT EXISTS)?
identifierList? #insertIntoTable
+ | INSERT INTO TABLE? multipartIdentifier REPLACE whereClause?
identifierList? #insertIntoReplaceWhere
Review Comment:
Oh, that is actually something I asked during our last chat. I think I still
have some confusion.
The grammar should be:
```
INSERT INTO TABLE? multipartIdentifier REPLACE whereClause(required)
identifierList(required)
```
as the unit test in this PR
```
INSERT INTO testcat.tbl REPLACE WHERE id = 3 SELECT * FROM source2
```
Following the notion that `? only means required` in the grammar, we should
remove both `?` and the grammar becomes
```
INSERT INTO TABLE? multipartIdentifier REPLACE whereClause identifierList
```
However, this will make the unit test fail, as identifierList's `?` is
removed
<img width="1548" alt="image"
src="https://user-images.githubusercontent.com/114777395/201408251-6d695422-f2d6-447e-9197-cd80c1b1e905.png";>
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:
##########
@@ -288,6 +289,11 @@ class AstBuilder extends SqlBaseParserBaseVisitor[AnyRef]
with SQLConfHelper wit
query,
overwrite = true,
ifPartitionNotExists)
+ case ctx: InsertIntoReplaceWhereContext =>
Review Comment:
Hmm, not fully understand but I think it might related to my previous
comment :)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
![https://user-images.githubusercontent.com/114777395/199851448-df273e7b-4a8b-4dc8-875f-b47882d8c265.png"](https://user-images.githubusercontent.com/114777395/199851448-df273e7b-4a8b-4dc8-875f-b47882d8c265.png"){kind=link}
![https://user-images.githubusercontent.com/114777395/201408251-6d695422-f2d6-447e-9197-cd80c1b1e905.png"](https://user-images.githubusercontent.com/114777395/201408251-6d695422-f2d6-447e-9197-cd80c1b1e905.png"){kind=link}