You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@shardingsphere.apache.org by "strongduanmu (via GitHub)" <gi...@apache.org> on 2026/10/10 01:39:47 UTC

[PR] Support additional MySQL SQL syntax and correct external parser inputs [shardingsphere]

strongduanmu opened a new pull request, #40120:
URL: https://github.com/apache/shardingsphere/pull/40120

   Changes proposed in this pull request:
     - Preserve the configured GitHub branch when loading external cases, restrict MySQL inputs to `.test` files, and exclude mysqltest connection commands, control directives, and file/script payloads. Respect routine delimiters, source character sets, and MySQL 8.0 executable-comment versions.
     - Support valid MySQL syntax observed in the external corpus: additional CREATE TABLE definitions and type aliases, CREATE FUNCTION/CREATE TRIGGER IF NOT EXISTS, INSTALL COMPONENT SET, LOAD XML ROWS IDENTIFIED BY, qualified identifiers and wildcards, account hosts, and SELECT function variants.
     - Correct charset/collation visitor construction, CONVERT USING parameters, and SQL_TSI interval mapping. Reuse the existing generic SQLStatement for GET DIAGNOSTICS.
     - Keep copied Doris grammar changes aligned and add paired parser SQL/AST cases with source-position and semantic assertions.
   
   The failing MySQL cases were identified from the [nightly SQL parser run](https://github.com/apache/shardingsphere/actions/runs/37822398925). mysqltest commands and payloads are filtered at input loading; valid SQL remains subject to parser and visitor validation.
   
   ### Validation
   
   - Full CI build: `./mvnw clean install -T1C -B -ntp -fae`.
   - All internal parser tests, including 1,939 MySQL and 1,411 Doris cases.
   - Complete `ExternalMySQLParserIT` against the configured MySQL 8.0 corpus: 64,910 cases, zero failures, errors, or skips.
   - Spotless and Checkstyle for every changed Java file; full Apache RAT check.
   - Remote Ubuntu benchmarks compare the original baseline with the final lexer/parser and visitor implementations using identical workloads, warmup, latency, and allocation measurements.
   
   ### Release Note
   
   Category: **Enhancement**
   
   Support additional MySQL CREATE TABLE, CREATE FUNCTION, CREATE TRIGGER, INSTALL COMPONENT, LOAD XML, GET DIAGNOSTICS and SELECT syntax.
   
   ---
   
   Before committing this PR, I'm sure that I have checked the following options:
   - [x] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
   - [x] I have self-reviewed the commit code.
   - [x] I have (or in comment I request) added corresponding labels for the pull request.
   - [x] I have passed maven check locally: `./mvnw clean install -T1C -B -ntp -fae`.
   - [x] I have made corresponding changes to the documentation.
   - [x] I have added corresponding parser integration tests for my changes.
   - [x] I have updated the Release Notes of the current development version. For more details, see [Update Release Note](https://shardingsphere.apache.org/community/en/involved/contribute/contributor/)
   - [ ] I have disclosed the tool and affected files or scope for any material AI assistance, as required by the [AI-assisted contribution policy].
   
   [AI-assisted contribution policy]: https://github.com/apache/shardingsphere/blob/master/AI_POLICY.md
   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Support additional MySQL SQL syntax and correct external parser inputs [shardingsphere]

Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.

strongduanmu commented on PR #40120:
URL: https://github.com/apache/shardingsphere/pull/40120#issuecomment-6096424803

• ### Result

**Review Result: Mergeable**

Both issues have been fixed. The latest upstream master has been merged, conflicts have been resolved, and all requested local checks have passed.

### Evidence

- The MySQL case loader preserves executable comments and SQL following ordinary block comments. It decodes latin1 using MySQL’s Windows-1252 mapping while preserving the five
special control bytes. No unit tests were added for the test infrastructure.
- The existing local cache was reused directly: 802 `.test` / `.result` pairs, totaling 1,604 files, all verified against official Git blob hashes. The existing loading, expected-
result filtering, parsing, and visitor flows ran in full, with input downloads supplied by a temporary cache provider.

| Verification | Result |
| --- | --- |
| MySQL external parser IT | 64,914 cases; 0 failures, 0 errors, 0 skips |
| All internal parser ITs | 21 test classes, 10,798 cases; 0 failures, 0 errors; 1 previously disabled case |
| Local CI | All 443 Maven projects succeeded; 28,668 tests, 0 failures, 0 errors, 149 existing skips |
| Spotless, Checkstyle, Apache RAT | Repository-wide checks passed; exit code 0 |

All 64,910 previously passing external cases were retained. Four previously omitted executable-comment cases were added, and five SQL text changes correspond to the corrected
latin1 decoding.

Internal IT command:
`./mvnw -pl test/it/parser -DskipITs -Dspotless.skip=true -Dtest=Internal*ParserIT test -B -ntp`

Local CI ran in an isolated snapshot whose 17,754 tracked files were byte-for-byte identical to the workspace:
`./mvnw clean install -T2 -B -ntp -fae -Dmaven.repo.local=<isolated-repository>`

Style and license command:
`./mvnw spotless:check checkstyle:check apache-rat:check -Pcheck -T2 -B -ntp`

### Coverage

This report covers the local PR candidate reviewed before commit and push: public head `6e3c02183681`, original merge-base `51a09f146116`, local merge commit `e0d283027490`
incorporating upstream master `70a17f43be57`, plus the then-uncommitted MySQL loader fix. The remote PR had not yet been updated at that point.

The review covered the public PR’s 61 files and the authorized upstream merge changes, totaling 99 files. It examined grammar and AST behavior, shared consumers, external loading,
internal assertions, and upstream merge content. Root-cause, blast-radius, runtime verification, and final convergence reviews were completed. The independent defensive-code review
passed, with no remaining blockers. Deletions were treated as expected, as confirmed by the user. The changes preserve the existing MySQL parsing contract without introducing a new
project commitment. CI coverage was local verification on JDK 21; remote Actions status was not rechecked.

--
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Support additional MySQL SQL syntax and correct external parser inputs [shardingsphere]

Posted by "FlyingZC (via GitHub)" <gi...@apache.org>.

FlyingZC merged PR #40120:
URL: https://github.com/apache/shardingsphere/pull/40120


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org