Skip to content

Conversation

@Mandukhai-Alimaa
Copy link

NULL VARBINARY values were incorrectly being converted to empty []byte{} instead of nil.

The root cause was that scanNullBytes() returned []byte directly, which when combined with Go's driver.Value interface conversion, lost the NULL semantics in certain scanning scenarios. (The issue was []byte(nil) getting converted to []byte{} through interface{} conversions.)

This change implements a proper NullBinary type following the same pattern as sql.NullString, sql.NullInt64, etc., ensuring consistent NULL handling:

  • Add NullBinary struct with Bytes []byte and Valid bool fields
  • Update scanNullBytes() to return NullBinary instead of []byte
  • Update ConvertValue for "varbinary" case to use NullBinary pattern

NULL values now properly return nil, empty values return []byte{}

Closes #166

@cla-bot
Copy link

cla-bot bot commented Nov 13, 2025

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@nineinchnick nineinchnick changed the title fix(trino): handle NULL VARBINARY values correctly in ConvertValue handle NULL VARBINARY values correctly in ConvertValue Nov 13, 2025
@nineinchnick nineinchnick added the enhancement New feature or request label Nov 13, 2025
@Mandukhai-Alimaa
Copy link
Author

Mandukhai-Alimaa commented Nov 17, 2025

Hi @nineinchnick, I sent the contributor license agreement 5 days ago. How do we know if it was successful or not? Also, is there anything else I need to do before this PR can be merged? Thanks so much.

@nineinchnick
Copy link
Member

CLA submissions are processed manually by Martin every two or three weeks. The last batch was done about two weeks ago. You don't have to do anything, I keep track of this, and will merge this PR once the CLA is processed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Development

Successfully merging this pull request may close these issues.

NULL VARBINARY values return empty []byte instead of nil

2 participants