Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix bigint unsigned null column scan to err type int64 #1612

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+5 −0
Open

fix bigint unsigned null column scan to err type int64 #1612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fb5a2a0
fix bigint unsigned null cloumn scan to err type int64
Jul 26, 2024
11f9287
add auth
Jul 26, 2024
4a5269c
use type sql.NullString support uint64, avoid out of range int64, bec…
Jul 29, 2024
fdd0987
Merge remote-tracking branch 'upstream/master' into fix_bigint_unsign…
methane Nov 11, 2024
6368cf3
use sql.Null
methane Nov 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ Zhang Xiang <angwerzx at 126.com>
Zhenye Xie <xiezhenye at gmail.com>
Zhixin Wen <john.wenzhixin at gmail.com>
Ziheng Lyu <zihenglv at gmail.com>
Jiabin Zhang <jiabin.z at qq.com>

# Organizations

Expand Down
4 changes: 4 additions & 0 deletions fields.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@
scanTypeInt64 = reflect.TypeOf(int64(0))
scanTypeNullFloat = reflect.TypeOf(sql.NullFloat64{})
scanTypeNullInt = reflect.TypeOf(sql.NullInt64{})
scanTypeNullUint = reflect.TypeOf(sql.Null[uint64]{})

Check failure on line 131 in fields.go

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 1.21, 9.0) 

undefined: sql.Null
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Breaking change: sql.Null requires Go 1.22.

As mentioned in the PR discussion, Go 1.21 support hasn't been dropped yet. Using sql.Null[uint64] would break compatibility with Go 1.21 and earlier versions.

Consider one of these alternatives:

  1. Use build tags to conditionally include this for Go 1.22+
  2. Create a custom NullUint64 type that implements the required interfaces:
// NullUint64 represents a uint64 that may be null
type NullUint64 struct {
    Uint64 uint64
    Valid  bool
}

// Implement Scanner and Valuer interfaces
func (n *NullUint64) Scan(value any) error {
    if value == nil {
        n.Uint64, n.Valid = 0, false
        return nil
    }
    n.Valid = true
    return convertAssign(&n.Uint64, value)
}

func (n NullUint64) Value() (driver.Value, error) {
    if !n.Valid {
        return nil, nil
    }
    return n.Uint64, nil
}

scanTypeNullTime = reflect.TypeOf(sql.NullTime{})
scanTypeUint8 = reflect.TypeOf(uint8(0))
scanTypeUint16 = reflect.TypeOf(uint16(0))
Expand Down Expand Up @@ -185,6 +186,9 @@
}
return scanTypeInt64
}
if mf.flags&flagUnsigned != 0 {
return scanTypeNullUint
}
Comment on lines +189 to +191
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Based on the test content examination, I can now provide a final response:

Test coverage for unsigned bigint nulls is insufficient

The codebase has tests for unsigned bigint (biguint) but only covers the non-null case with basic values (0, 65535, 42). There are no specific test cases for:

  • Null unsigned bigint values
  • Edge cases near uint64 maximum values
  • Values exceeding int64 range
  • Add test cases in driver_test.go for scanTypeNullUint similar to existing bigintnull tests
  • Include edge cases testing uint64 values > math.MaxInt64
🔗 Analysis chain

Verify test coverage for unsigned bigint nulls.

Let's ensure the changes are properly tested, especially for edge cases where uint64 values exceed int64 range.

Let's examine the specific test content in driver_test.go since it appears in both searches.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for tests involving unsigned bigint columns
echo "Searching for tests with unsigned bigint columns..."
rg -l --type go "(?i)(unsigned.*bigint|uint64.*null)" *_test.go

# Search for existing null scanning tests
echo "Searching for null scanning tests..."
rg -l --type go "(?i)(nullint|nulluint|null.*scan)" *_test.go

Length of output: 755

Script:

#!/bin/bash
# Check the content of driver_test.go for unsigned bigint and null scanning tests
echo "Examining driver_test.go for unsigned bigint and null scanning tests..."
rg -A 10 -B 2 "(?i)(unsigned.*bigint|uint64.*null|nullint|nulluint|null.*scan)" driver_test.go

Length of output: 11427

return scanTypeNullInt

case fieldTypeFloat:
Expand Down
Loading