Skip to content

fix: validate PROTO/ENUM wire kind in literal cast plugins#164

Merged
apstndb merged 1 commit into
mainfrom
fix/issue-156-proto-enum-wire-validation
Jun 8, 2026
Merged

fix: validate PROTO/ENUM wire kind in literal cast plugins#164
apstndb merged 1 commit into
mainfrom
fix/issue-156-proto-enum-wire-validation

Conversation

@apstndb

@apstndb apstndb commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • Call requireStringWire in FormatProtoAsCast and FormatEnumAsCast after NULL handling so malformed structpb kinds return ErrUnknownType instead of emitting invalid SQL.
  • Add regression tests for non-string wire kinds (bool/number) on PROTO and ENUM via LiteralFormatConfig.

Fixes #156.

Test plan

  • make check
  • New TestFormatProtoEnumCastRejectsNonStringWire covers PROTO/ENUM bool and number wire kinds

Made with Cursor

FormatProtoAsCast and FormatEnumAsCast now call requireStringWire after
NULL handling so malformed structpb kinds return ErrUnknownType instead
of emitting invalid SQL. Fixes #156.

Co-authored-by: Cursor <cursoragent@cursor.com>

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request adds validation checks in common.go to ensure that Proto and Enum values are represented as string wire types before formatting them as casts, using requireStringWire in FormatProtoAsCast and FormatEnumAsCast. It also adds unit tests in common_test.go to verify that non-string wire types are correctly rejected with ErrUnknownType. There are no review comments, and I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@apstndb apstndb left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

LGTM. Calling requireStringWire after the IsNull guard in FormatProtoAsCast/FormatEnumAsCast is the right placement, and returning ErrUnknownType for non-string PROTO/ENUM wire matches how validateScalarWire already classifies those codes (both route to requireStringWire), so the slow/scalar paths stay consistent. Previously a bool/number wire would silently produce CAST(b"" AS ...) / CAST( AS ...) invalid SQL, so this is a clear correctness fix. The table test covers bool and number kinds for both PROTO and ENUM via LiteralFormatConfig. No issues found.

@apstndb apstndb merged commit 3e9b466 into main Jun 8, 2026
2 checks passed
@apstndb apstndb deleted the fix/issue-156-proto-enum-wire-validation branch June 8, 2026 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

format: validate PROTO/ENUM wire kind in literal cast plugins

1 participant