RaigorJiang commented on PR #38687:
URL: https://github.com/apache/shardingsphere/pull/38687#issuecomment-4427259062
• Merge Verdict: Mergeable
Reviewed Scope
- parser/distsql/engine/src/main/antlr4/imports/BaseRule.g4
- parser/distsql/engine/src/main/antlr4/imports/Literals.g4
- parser/distsql/engine/src/main/antlr4/imports/RDLStatement.g4
- parser/distsql/engine/src/main/antlr4/imports/RALStatement.g4
- parser/distsql/engine/src/main/antlr4/imports/RQLStatement.g4
- proxy/backend/core/.../ShowTableMetaDataExecutor.java
- proxy/backend/core/.../RefreshTableMetaDataExecutor.java
- mode/core/.../ContextManager.java
- test/it/parser parser cases, assertions, and unsupported cases
- DistSQL docs for SHOW/REFRESH TABLE METADATA and RDL storage unit
statements
- Official PostgreSQL / MySQL / MariaDB documentation for identifier and
string literal semantics
Not Reviewed Scope
- Other non-DistSQL parser families
- Other DistSQL commands not touched by this PR
- Live end-to-end validation against real PostgreSQL/MySQL/MariaDB
instances
Need Expert Review
- No
Basis
- The root cause is fixed end to end. SHOW/REFRESH TABLE METADATA now use
distSQLIdentifier for object-name positions, and the execution path preserves
IdentifierValue semantics
through statement, executor, and ContextManager.
- The grammar split is consistent and low risk:
- IDENTIFIER_ keeps bare identifiers and backtick-quoted identifiers
- DOUBLE_QUOTED_TEXT is shared by identifier and string positions
where the DistSQL syntax intentionally allows it
- SINGLE_QUOTED_TEXT remains the single-quoted string token
- The change does not introduce a lexer conflict. DOUBLE_QUOTED_TEXT is
reused by different parser rules in different semantic positions, which is
valid and intentional.
- Adjacent paths were checked:
- RDL storage unit statements still accept double-quoted string values
- SHOW DIST VARIABLE / SET DIST VARIABLE remain on their existing
identifier/string semantics
- single-quoted object names are rejected as expected
- Official database documentation supports the behavior boundary:
- PostgreSQL uses double quotes for quoted identifiers and single
quotes for string literals
- MySQL/MariaDB support backticks for identifiers, and double quotes
for strings unless ANSI_QUOTES changes that behavior
- Parser IT, unit tests, checkstyle, and spotless all passed on the latest
head.
Pre-merge Checks
- ./mvnw -pl test/it/parser -am -DskipITs -Dspotless.skip=true
-Dtest=InternalDistSQLParserIT test -Dsurefire.failIfNoSpecifiedTests=false
Exit code: 0
- ./mvnw checkstyle:check -Pcheck -T1C -pl
parser/distsql/engine,test/it/parser -am
Exit code: 0
- ./mvnw spotless:check -Pcheck -T1C -pl
parser/distsql/engine,test/it/parser -am
Exit code: 0
- ./mvnw -pl proxy/backend/core,mode/core -am -DskipITs
-Dspotless.skip=true
-Dtest=ShowTableMetaDataExecutorTest,RefreshTableMetaDataExecutorTest,DistSQLUpdateProxyBackendHandlerTest,DistSQLQueryProxyBackendHandlerTest,ContextManagerTest
test
-Dsurefire.failIfNoSpecifiedTests=false
Exit code: 0
--
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]
