Fix int64/uint64 type inference to use BIGINT instead of INTEGER#313
Closed
vikrantpuppala wants to merge 1 commit intodatabricks:mainfrom
Closed
Fix int64/uint64 type inference to use BIGINT instead of INTEGER#313vikrantpuppala wants to merge 1 commit intodatabricks:mainfrom
vikrantpuppala wants to merge 1 commit intodatabricks:mainfrom
Conversation
This fixes a bug where int64 and uint64 values were incorrectly mapped to SqlInteger instead of SqlBigInt, causing insert failures for large values with error: INVALID_PARAMETER_MARKER_VALUE.INVALID_VALUE_FOR_DATA_TYPE Changes: - int64 now uses strconv.FormatInt() and maps to SqlBigInt - uint64 now maps to SqlBigInt - Added safe type assertion in convertNamedValuesToSparkParams to prevent panic when Parameter.Value is not a string Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This pull request fixes a critical bug where int64 and uint64 values were incorrectly mapped to SqlInteger (INT) instead of SqlBigInt (BIGINT), causing data truncation and server rejection of large values.
Changes:
- Fixed int64 type inference to use
strconv.FormatIntand SqlBigInt instead of lossy int casting - Fixed uint64 type inference to use SqlBigInt and removed redundant type cast
- Added defensive type assertion in
convertNamedValuesToSparkParamsto handle non-string Parameter values
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| parameters.go | Fixed int64/uint64 type mappings from INTEGER to BIGINT and corrected value conversion to prevent data loss; added safe type handling for Parameter.Value |
| parameter_test.go | Added comprehensive test suite covering int64/uint64 BIGINT inference, edge cases (min/max values), explicit Parameter handling, and int32 preservation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Collaborator
Author
|
Closing in favor of #316 which includes this fix along with the float64 fix |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
strconv.Itoa(int(value))which truncates large valuesconvertNamedValuesToSparkParamsto prevent panic whenParameter.Valueis not a stringProblem
When inserting int64/uint64 values into BIGINT columns, the driver was sending them with type
INTEGERinstead ofBIGINT, causing the server to reject large values with error:Test plan
TestParameter_BigInt)🤖 Generated with Claude Code