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

Posted to notifications@shardingsphere.apache.org by "sushu0 (via GitHub)" <gi...@apache.org> on 2026/10/19 17:06:06 UTC

[PR] Fix Firebird KEY, TEXT and COMMENT column aliases [shardingsphere]

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

Fixes #40262.

Changes proposed in this pull request:
- Allow `KEY`, `TEXT`, and non-leading `COMMENT` tokens through Firebird's existing `unreservedWord` identifier rule, so they can be used as unquoted column aliases with or without `AS`.
- Add six Firebird-only Parser IT cases that assert the parsed column, alias and source bounds, and add the required release-note entry.

For example, `SELECT order_id AS KEY FROM t_order` currently raises `SQLParsingException`; after this change it yields a column projection with alias `KEY`. The same correction covers `TEXT`, `COMMENT`, and implicit aliases. No lexer token or visitor implementation changes are needed.

The [Firebird 5.0 SELECT reference](https://www.firebirdsql.org/file/documentation/html/en/refdocs/fblangref50/firebird-50-language-reference.html#fblangref50-dml-select-column-list) allows aliases with optional `AS`. In the versioned engine grammar, [`KEY` is non-reserved](https://github.com/FirebirdSQL/firebird/blob/v5.0.4/src/dsql/parse.y#L9076), and aliases use [the normal name rule](https://github.com/FirebirdSQL/firebird/blob/v5.0.4/src/dsql/parse.y#L8860-L8863). `TEXT` is handled as a symbol. `COMMENT` is listed as reserved in the manual, but the [engine lexer recognizes its keyword token only at the start of a statement](https://github.com/FirebirdSQL/firebird/blob/v5.0.4/src/dsql/Parser.cpp#L1212-L1226); at alias positions it is a normal symbol. This patch keeps `COMMENT ON`, key constraints and `BLOB SUB_TYPE TEXT` keyword handling intact.

Validation with Java 21 and Maven 3.9.16:
- Baseline dependency setup: `mvn -B -ntp -T2 -pl test/it/parser -am -DskipTests -Dspotless.skip=true -Dcheckstyle.skip=true install` — passed; setup only, tests skipped.
- Before the grammar fix: `mvn -B -ntp -pl test/it/parser -Dit.test=InternalFirebirdParserIT verify` — 264 supported Firebird cases, exactly the six new alias cases errored, existing 258 passed; 221 module unit tests passed.
- After the fix: `mvn -B -ntp -pl parser/sql/engine/dialect/firebird install` — passed and regenerated the Firebird parser.
- `mvn -B -ntp -pl test/it/parser -Dit.test=InternalFirebirdParserIT,InternalUnsupportedFirebirdParserIT verify` — passed: 264 supported Firebird cases, 1 unsupported case, 221 module unit tests; no failures, errors or skips. Existing cases cover `COMMENT ON`, key constraints and `BLOB SUB_TYPE TEXT`.
- `git diff --check` and XML well-formedness checks passed. The changed G4, SQL/case XML and Markdown files are outside the configured Spotless file types and default Checkstyle Java/properties includes; no new files or license headers were added/changed.

The full-repository `./mvnw clean install -B -T1C -Pcheck` and a live Firebird server/Proxy comparison were not run. This is parser-level reproduction and regression evidence.

AI assistance and draft status:
Codex materially assisted with investigation, the grammar change, the six Parser IT cases, the release-note entry, verification and this description. Affected files are:
- `parser/sql/engine/dialect/firebird/src/main/antlr4/imports/firebird/BaseRule.g4`
- `test/it/parser/src/main/resources/sql/supported/dml/select.xml`
- `test/it/parser/src/main/resources/case/dml/select.xml`
- `RELEASE-NOTES.md`

The human contributor has not yet personally reviewed and confirmed understanding of this patch. This PR is intentionally Draft pending that review and is not ready for merge. Automated/agent review is not a substitute for the human responsibilities in `AI_POLICY.md`.

Requested labels: `type: bug`, `in: SQL parse`, `db: Firebird`.

---

Before submitting this PR, I confirm that:
- [ ] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
- [ ] I have reviewed my changes.
- [x] I have (or in comment I request) added corresponding labels for the pull request.
- [ ] I have passed the required local checks: `./mvnw clean install -B -T1C -Pcheck`.
- [x] I have updated the relevant documentation, if applicable.
- [x] I have added or updated unit tests for my changes, if applicable. Parser/visitor coverage is in the required Parser IT: six integration cases added; no unit-test changes.
- [x] I have updated the Release Notes of the current development version, if applicable.
- [x] I have disclosed the tool and affected files or scope for any material AI assistance, as required by the [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] Fix Firebird KEY, TEXT and COMMENT column aliases [shardingsphere]

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

terrymanu merged PR #40283:
URL: https://github.com/apache/shardingsphere/pull/40283


-- 
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] Fix Firebird KEY, TEXT and COMMENT column aliases [shardingsphere]

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

sushu0 commented on PR #40283:
URL: https://github.com/apache/shardingsphere/pull/40283#issuecomment-6283330889

   Technical update for unchanged head `e11ff32d`: Codex completed the full local `-Pcheck` validation: all 443 reactor modules succeeded, with no failed/skipped modules (47m23s). Both checkouts stayed clean and the four reviewed file hashes are unchanged. The exact command, resource limits and read-only Spotless verification are documented in the updated description. Hosted workflows still require maintainer approval; no hosted CI pass is claimed. Please approve the workflows when convenient. Thank you.
   


-- 
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