AppendRow unset-version switch matches string and []byte, but not *string. The reflect-based []byte check only catches slice-of-uint8, so *string falls through to appendRowObject, where no branch converts a pointer-to-string into JSON.
obj stays nil and is replaced with an empty chcol.NewJSON(). Then AppendRow returns nil, the caller sees no error, and the column is stored as {}.
I think this bug is made visible after this PR (which changed the order of append. first try object then string. If like before the PR first trying string would have worked)
Below is a simple example to reproduce on tip of the main branch
func bug(ctx context.Context, conn driver.Conn) error {
if err := conn.Exec(ctx, "DROP TABLE IF EXISTS go_json_repro_issue1"); err != nil {
return err
}
if err := conn.Exec(ctx, `CREATE TABLE go_json_repro_issue1 (product JSON) ENGINE=Memory`); err != nil {
return err
}
jsonText := `{"id":1234,"name":"Book","tags":["a","b"]}`
ptr := &jsonText
batch, err := conn.PrepareBatch(ctx, "INSERT INTO go_json_repro_issue1 (product)")
if err != nil {
return err
}
if err := batch.Append(ptr); err != nil {
return fmt.Errorf("append returned an error (would be preferable): %w", err)
}
if err := batch.Send(); err != nil {
return err
}
var stored string
if err := conn.QueryRow(ctx,
"SELECT toString(product) FROM go_json_repro_issue1").Scan(&stored); err != nil {
return err
}
fmt.Printf(" input (*string): %s\n", jsonText)
fmt.Printf(" stored value: %s\n", stored)
fmt.Printf(" data lost: %t\n", stored != jsonText)
return nil
}
This will produce following output
$ go run json_issue.go
input (*string): {"id":1234,"name":"Book","tags":["a","b"]}
stored value: {}
data lost: true
Proposal:
- Handle
*string case in both Append() and AppendRow() similar to string and []byte. Doing so it invokes correct appendRowString() api for string.
- Check other places in JSON where this silent failure can happen if type missaligned. And throw explicit error instead
AppendRowunset-version switch matchesstringand[]byte, but not*string. The reflect-based[]bytecheck only catches slice-of-uint8, so*stringfalls through toappendRowObject, where no branch converts a pointer-to-string into JSON.objstays nil and is replaced with an emptychcol.NewJSON(). ThenAppendRowreturns nil, the caller sees no error, and the column is stored as{}.I think this bug is made visible after this PR (which changed the order of append. first try object then string. If like before the PR first trying string would have worked)
Below is a simple example to reproduce on tip of the main branch
This will produce following output
Proposal:
*stringcase in both Append() and AppendRow() similar tostringand[]byte. Doing so it invokes correctappendRowString()api for string.