Skip to content

Parse SQL92 create column nullability - #39439

Open
iamhucong wants to merge 2 commits into
apache:masterfrom
iamhucong:fix-sql92-create-column-nullability-parser
Open

Parse SQL92 create column nullability#39439
iamhucong wants to merge 2 commits into
apache:masterfrom
iamhucong:fix-sql92-create-column-nullability-parser

Conversation

@iamhucong

Copy link
Copy Markdown
Contributor

Fix SQL92 CREATE TABLE column nullability metadata.

Problem

The SQL92 grammar accepts column-level NOT NULL and NULL options, but SQL92DDLStatementVisitor.visitColumnDefinition() always constructed ColumnDefinitionSegment with notNull=false.

Change

  • Derive notNull from the parsed NOT NULL option.
  • Add a focused SQL92 parser case covering NOT NULL and explicit NULL.
  • Add the ShardingSphere 5.5.4 bug-fix release note.

Verification

  • Baseline: InternalSQL92ParserIT passed 207 tests.
  • InternalSQL92ParserIT: 208 tests passed after the change.
  • SQL92 parser module passed.
  • Spotless passed.
  • Checkstyle reported 0 violations.
  • git diff --check passed.

WHAT: Preserve SQL92 CREATE TABLE column nullability metadata.

WHY: The grammar accepted NOT NULL and NULL options, but the visitor always reported columns as nullable.

HOW: Derive the segment metadata from the parsed nullability option and add focused SQL92 regression coverage.
@iamhucong

Copy link
Copy Markdown
Contributor Author

Code Correctness Review

Review Result: Mergeable

PR #39439 correctly preserves SQL92 CREATE TABLE column nullability metadata. No code-correctness blockers were found.

Evidence

  • Reviewed the latest public head 464028e2dd9dc9878dc6652dbb3618e4de2316f6.
  • The SQL92 grammar accepts NOT NULL and NULL through dataTypeOption at parser/sql/engine/dialect/sql92/src/main/antlr4/imports/sql92/DDLStatement.g4:68-80.
  • SQL92DDLStatementVisitor.visitColumnDefinition() now derives notNull from the parsed NOT NULL option at SQL92DDLStatementVisitor.java:115-125.
  • Explicit NULL and columns without a nullability option remain notNull=false.
  • The parser test covers both boundaries through create_table_with_explicit_column_nullability_sql92:
    • order_id INT NOT NULL
    • user_id INT NULL
  • ColumnDefinitionSegmentBinder preserves the metadata through binding.
  • Verification on the reviewed head:
    • SQL92 parser module passed.
    • InternalSQL92ParserIT: 208 tests passed.
    • Spotless passed.
    • Checkstyle reported 0 violations.
    • git diff --check passed.

Coverage

  • Base: 415e37ead38c6b3e5e39127e23cdf4de47032e11.
  • GitHub's authoritative file list matches the local triple-dot scope: 4 files, +14/-2.
  • Reviewed RELEASE-NOTES.md, SQL92DDLStatementVisitor.java, the SQL92 supported SQL cases, and the SQL92 expected parser cases.
  • Completed root-cause and behavior, blast-radius and contracts, and tests/runtime/operations discovery.
  • Checked the SQL92 parser facade, grammar option boundaries, shared binder path, ColumnDefinitionSegment, and the Encrypt CREATE TABLE consumer.
  • SQL92 grammar, visitor behavior, parser assertions, release note, and standard references are consistent.
  • No unresolved evidence or coverage gap remains; Completion Gate passed.

This is a code-scope review; CI was not reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant