Skip to content

dolthub/dolt#9425 - Fix enum zero validation in strict mode#3076

Merged
elianddb merged 21 commits intomainfrom
elianddb/9425-enum-zero-strict-mode
Jul 10, 2025
Merged

dolthub/dolt#9425 - Fix enum zero validation in strict mode#3076
elianddb merged 21 commits intomainfrom
elianddb/9425-enum-zero-strict-mode

Conversation

@elianddb
Copy link
Copy Markdown
Contributor

@elianddb elianddb commented Jul 8, 2025 •
edited
Loading

Fixes dolthub/dolt#9425:

  • Add strict mode check for 0 values in EnumType.Convert()
  • Return data truncation error for invalid 0 values in strict mode
  • Allow 0 values when empty string is explicitly defined as enum value
  • Add row number tracking in insertIter for accurate error reporting
  • Enhance enum errors with column name and row number
  • Fix ErrInvalidType formatting issues in enum expression

🤖 Generated with Claude Code

@elianddb elianddb force-pushed the elianddb/9425-enum-zero-strict-mode branch from 925b0e8 to 77b4675 Compare July 8, 2025 21:15
jycor
jycor reviewed Jul 9, 2025
View reviewed changes
jycor
jycor reviewed Jul 9, 2025
View reviewed changes
@elianddb elianddb force-pushed the elianddb/9425-enum-zero-strict-mode branch from 9fbc47b to 5ea6b1a Compare July 10, 2025 00:08
elianddb and others added 21 commits July 10, 2025 09:29
@elianddb @claude
dolthub/dolt#9425 - Fix enum zero validation in strict mode
b81d549 
- Add strict mode check for 0 values in EnumType.Convert()
- Return data truncation error for invalid 0 values in strict mode
- Allow 0 values when empty string is explicitly defined as enum value
- Add row number tracking in insertIter for accurate error reporting
- Enhance enum errors with column name and row number
- Fix ErrInvalidType formatting issues in enum expression
- Add comprehensive test cases for strict/non-strict modes

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elianddb @claude
Fix enum zero strict mode validation to match MySQL exactly
5d8690a 
After MySQL comparison server testing, corrected logic to reject 0 values
in strict mode regardless of enum definition, matching MySQL behavior exactly.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elianddb @claude
Fix CREATE TABLE enum default validation to return correct error type
1f84fda 
Modified columndefault.go to detect enum data truncation errors and return
ErrInvalidColumnDefaultValue instead of ErrIncompatibleDefaultType to match
MySQL behavior exactly.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elianddb @claude
Fix CREATE TABLE enum default validation to show proper column names
3cb9fee 
- Add early validation in tableSpecToSchema to catch enum default 0 before conversion
- Prevents columndefault.go from returning "(unknown)" in error messages
- Enhanced validateDefaultExprs to handle enum defaults with proper column context
- Now matches MySQL behavior exactly: "Invalid default value for 'column_name'"

Resolves the remaining column name issue for CREATE TABLE statements
while maintaining all existing INSERT validation functionality.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elianddb
[ga-format-pr] Run ./format_repo.sh to fix formatting
049e017
@elianddb
Fix sql_mode detection in enum strict mode validation
57d1bbb 
Use ctx.GetSessionVariable() instead of ctx.Session.GetSessionVariable()
to properly access session variables in the enum conversion context.
@elianddb
Fix INSERT IGNORE enum zero handling in strict mode
1c9d6d9 
Allow uint16 enum indices to be converted without strict mode validation.
This fixes INSERT IGNORE behavior where stored uint16(0) values should
display as empty strings, matching MySQL behavior exactly.

Resolves the (unknown) error during SELECT after INSERT IGNORE.
@elianddb @claude
Eliminate fragile string-matching enum error enhancement
dfe60a0 
Previously used fragile string matching to detect enum data truncation errors:
- if types.IsEnum(col.Type) && strings.Contains(cErr.Error(), "Data truncated for column")

Now enum Convert method returns ErrConvertingToEnum directly for invalid 0 values in strict mode,
which gets properly enhanced with column name and row number via existing error handling pattern:
- else if types.ErrConvertingToEnum.Is(cErr)

This eliminates the fragile string-parsing approach while maintaining exact same functionality
and MySQL-compatible error messages with proper column names and row numbers.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elianddb @claude
Clean up redundant string comparisons and imports
15d9142 
- Remove unused isInsertContext function from enum.go
- Fix import cycle by removing unnecessary types import from columndefault.go
- Restore necessary DDL validation for proper column name error reporting
- Maintain all functionality while eliminating fragile string matching patterns

All tests pass and MySQL comparison server behavior matches exactly.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elianddb
[ga-format-pr] Run ./format_repo.sh to fix formatting
1059493
@elianddb
rm Skip var
a135718
@elianddb
rm reduundant GetSessionVariable
5b77261
@elianddb @claude
Add STRICT_ALL_TABLES query test for enum zero validation
997b8cb 
- Add comprehensive test case "enums with zero strict all tables"
- Tests single row insert, multi-row insert, and CREATE TABLE default scenarios
- Ensures both STRICT_TRANS_TABLES and STRICT_ALL_TABLES modes are covered
- Complements existing STRICT_TRANS_TABLES test for complete coverage

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elianddb
rm extra comment
6069cfa
@elianddb
fix format errs and redundant edits
dd2b3a9
@elianddb @claude
Address PR review feedback from jycor
82d22c0 
- Use SqlMode struct instead of manual string parsing in isStrictMode
- Leverage LoadSqlMode() and ModeEnabled() methods for proper SQL mode detection
- Unskip enum default validation tests that now work with our implementation
- Update expected error types to ErrInvalidColumnDefaultValue for enum defaults

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elianddb
fix validateDefaultExprs()
de3c27c
@elianddb
add str from e
a1cecc8
@elianddb @claude
Fix enum literal default validation to match MySQL behavior
121b5f3 
- Add validateEnumLiteralDefault method for stricter enum default validation
- Reject numeric index references ('1', 1) for literal enum defaults
- Allow only exact enum value matches for literal defaults
- Preserve original ErrInvalidType format in enum expression
- Fixes enums_with_default_values test failures

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elianddb @claude
Fix enum default validation to use actual column names
16ef988 
- Move validateEnumLiteralDefault to resolve_column_defaults.go
- Pass actual column name instead of '(unknown)' to match MySQL
- MySQL returns 'Invalid default value for 'column_name'' format
- Remove enum validation from CheckType to avoid duplicate logic
- Verified behavior matches MySQL exactly for both invalid cases

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elianddb
cleanup code
24ee22c
@elianddb elianddb force-pushed the elianddb/9425-enum-zero-strict-mode branch from 5ea6b1a to 24ee22c Compare July 10, 2025 16:30
@elianddb elianddb requested a review from jycor July 10, 2025 17:30
jycor
jycor approved these changes Jul 10, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@jycor jycor left a comment

Choose a reason for hiding this comment

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

LGTM

@elianddb elianddb merged commit 8b0896d into main Jul 10, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jycor jycor jycor approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

0 value is not allowed for enum

2 participants

@elianddb @jycor