[SPARK-29680][SQL][Followup] Replace qualifiedName with multipartIdentifier #26419
Conversation
|
actually I am not sure if this should keep as followup or should create another JIRA ticket. @cloud-fan |
|
Test build #113360 has finished for PR 26419 at commit
|
viirya
changed the title
viirya
force-pushed
the
SPARK-29680-followup2
branch
from
5b99355
to
c8fd004
Compare
viirya
changed the title
| @@ -136,20 +136,22 @@ statement | |||
| | ALTER TABLE multipartIdentifier | |||
| ADD (COLUMN | COLUMNS) | |||
| '(' columns=qualifiedColTypeWithPositionList ')' #addTableColumns | |||
| | ALTER TABLE table=multipartIdentifier | |||
| RENAME COLUMN | |||
| from=multipartIdentifier TO to=multipartIdentifier #renameTableColumn | |||
There was a problem hiding this comment.
to should be a single identifer. We can only rename a column or a field, but not a field chain.
There was a problem hiding this comment.
oh, then it should be errorCapturingIdentifier.
| @@ -802,7 +808,7 @@ intervalUnit | |||
| ; | |||
|
|
|||
| colPosition | |||
| : FIRST | AFTER qualifiedName | |||
There was a problem hiding this comment.
can we remove the definition of qualifiedName if it's used nowhere?
There was a problem hiding this comment.
in current change i just replaced qualifiedName for DDL commands.
there are still few places using qualifiedName. Need to address them see if we can replace them too.
There was a problem hiding this comment.
Can a function be named as test-func without quote?
There are rules CRATE (OR REPLACE)? TEMPORARY? FUNCTION... and DROP TEMPORARY? FUNCTION that use qualifiedName.
There was a problem hiding this comment.
qualifiedName can't match a-b. I take another look, qualifiedName is still needed in places where - is a legal following, e.g. select a - b, we must use qualifiedName to match column name.
For function name in CREATE FUNCTION, we can use multipartIdentifier
There was a problem hiding this comment.
If we use multipartIdentifier in CREATE FUNCTION, meaning function name is multipartIdentifier. Will it be a problem for parsing SELECT a-test_func(...) ...?
In this case, a-test will be parsed by multipartIdentifier and an exception will be thrown to ask adding quote for it.
There was a problem hiding this comment.
We can only use multipartIdentifier when define the function, e.g. CREATE FUNCTION ..., not when calling the function.
|
Test build #113362 has finished for PR 26419 at commit
|
|
Test build #113397 has finished for PR 26419 at commit
|
| """ | ||
| |CREATE TABLE IF NOT EXISTS tab | ||
| |USING test-provider | ||
| |COMMENT 'This is the staging page view table' |
There was a problem hiding this comment.
nit: we don't have to specify COMMENT, LOCATION, etc to test provider name.
|
Test build #113442 has finished for PR 26419 at commit
|
|
Test build #113465 has finished for PR 26419 at commit
|
|
Thanks. Merging to master. |
What changes were proposed in this pull request?
Replace qualifiedName with multipartIdentifier in parser rules of DDL commands.
Why are the changes needed?
There are identifiers in some DDL rules we use
qualifiedName. We should usemultipartIdentifierbecause it can capture wrong identifiers such astest-table,test-col.Does this PR introduce any user-facing change?
Yes. Wrong identifiers such as test-table, will be captured now after this change.
How was this patch tested?
Unit tests.