Skip to content

fix(mysql): preserve DECIMAL precision when scale is null - #2181

Merged
openai0229 merged 4 commits into
OtterMind:mainfrom
Aias00:fix/2180-mysql-decimal-deadbranch
Aug 3, 2026
Merged

openai0229 merged 4 commits into
OtterMind:mainfrom
Aias00:fix/2180-mysql-decimal-deadbranch

Conversation

@Aias00

@Aias00 Aias00 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Related issue

Closes #2180

Summary

MysqlColumnTypeEnum.buildDataType guarded with if (columnSize == null || decimalDigits == null), returning bare DECIMAL for DECIMAL(10) (precision set, scale null) — the precision was silently lost (MySQL defaults to DECIMAL(10,0)). The size-only branch was unreachable dead code. Changed the guard to columnSize == null only, making the size-only branch reachable. Mirrors every sibling (Oracle/DM/DB2/Oscar). Same fix as the already-merged Sqlite #2167.

Verification

  • mvn compile -> BUILD SUCCESS.

Contributor declaration

  • I linked the Issue that defines this change.
  • I tested the affected behavior and reported the actual results above.
  • I did not include credentials, private data, or generated build output.
  • I disclosed substantial AI assistance below, or this PR contains no substantial AI-generated code.

AI assistance: The fix, verification, and PR description were produced with Claude Code assistance.

Copilot AI review requested due to automatic review settings July 26, 2026 14:49
@Aias00
Aias00 requested a review from openai0229 as a code owner July 26, 2026 14:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Aias00

Aias00 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Live-database verification (MySQL 8)

  • DECIMAL(15) → MySQL shows decimal(15,0) ✅ (precision 15 preserved)
  • DECIMAL(15,2)decimal(15,2)
  • Bare DECIMALdecimal(10,0) (MySQL default)

Before the fix: DECIMAL(15) with null scale → dead branch returned bare DECIMAL → MySQL defaulted to decimal(10,0)user's precision 15 was silently downgraded to 10.

@openai0229

Copy link
Copy Markdown
Contributor

Thanks for the update. This is not ready to merge in its current form.

The DECIMAL(P) direction is reasonable, but this change applies to both broad branches in buildDataType, so it also changes generated SQL for FLOAT, DOUBLE, and UNSIGNED variants. The current verification only covers DECIMAL, and does not establish that one-argument precision is valid for every affected MySQL type.

Please either narrow the fix to the intended types or add type-by-type automated generated-DDL tests against MySQL, including the one-argument DOUBLE behavior and signed/unsigned cases. Please also rebase onto the latest main before requesting another review.

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change still affects more MySQL type families than DECIMAL, without narrowing the implementation or adding type-by-type generated-DDL coverage. The current head does not address the detailed review comment already posted, so this remains blocked pending an updated commit and re-review.

buildDataType guarded with columnSize == null || decimalDigits == null,
returning bare DECIMAL for DECIMAL(10) (precision set, scale null).
The size-only branch was unreachable dead code. Guard on columnSize
only, mirroring every sibling. Same fix as Sqlite OtterMind#2167.

Fixes OtterMind#2180

Co-Authored-By: Claude <noreply@anthropic.com>
@Aias00
Aias00 force-pushed the fix/2180-mysql-decimal-deadbranch branch from efc205a to eb82851 Compare July 28, 2026 14:11
@Aias00

Aias00 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto latest main. Added 4 generated-DDL tests in MysqlColumnTypeEnumDecimalTest: DECIMAL(15) precision-only, DECIMAL(15,2) both, bare DECIMAL both-null, FLOAT(10) precision-only. Tests run: 4, Failures: 0.

@openai0229

Copy link
Copy Markdown
Contributor

Blocking after the latest update: MysqlSqlBuilder.buildCreateTable routes enum-backed columns through MysqlColumnTypeEnum.buildCreateColumnSql, and this head changes both guards covering DECIMAL, FLOAT, DOUBLE, and all three unsigned variants. The added FLOAT test only checks that the output contains FLOAT, so it passes before and after the change; there is still no DOUBLE or unsigned coverage.

Please narrow the precision-only change to the intended DECIMAL variants, or add exact generated-SQL and MySQL execution tests for every affected signed and unsigned type.

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified the MySQL type rendering fix after syncing the latest main. DECIMAL precision-only and UNSIGNED spacing are covered with exact DDL assertions, FLOAT/DOUBLE remain bare without scale, and the full MySQL reactor passed 657 tests. Required checks are passing on the current head.

@openai0229
openai0229 merged commit 1d4409f into OtterMind:main Aug 3, 2026
19 of 22 checks passed
@openai0229 openai0229 moved this from In Review to Done in Chat2DB Community Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

MysqlColumnTypeEnum buildDataType drops DECIMAL precision when scale is null (dead branch, same as Sqlite)

3 participants