Skip to content

feat(go): clarify bind behavior#4267

Merged
lidavidm merged 2 commits intoapache:mainfrom
lidavidm:clarify-go-bind
Apr 27, 2026
Merged

feat(go): clarify bind behavior#4267
lidavidm merged 2 commits intoapache:mainfrom
lidavidm:clarify-go-bind

Conversation

@lidavidm
Copy link
Copy Markdown
Member

I noticed the drivers were inconsistent. This tries to be more explicit about what you are expected to do (bind, then release) and matches the behavior of the Snowflake driver.

@lidavidm lidavidm requested a review from amoeba April 24, 2026 01:48
Comment thread go/adbc/adbc.go Outdated
Comment on lines +743 to +744
// driver will take ownership of (Retain) the given batch; the
// application may Release the batch after binding. The driver will
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

because the driver calls Retain it is the application's responsibility that they have to call Release separately otherwise it'll be leaked (as opposed to "may")

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll change it to "must still" (I was thinking of the case that the application wants to keep the batch around for other uses...but even so eventually it must call Release)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll also simplify to "the driver will Retain the batch" since "ownership" isn't quite a thing in Go-speak

Comment thread go/adbc/adbc.go Outdated
Comment on lines +753 to +754
// driver will take ownership of (Retain) the given reader; the
// application may Release the reader after binding. The driver will
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same as above, the application must call Release

I noticed the drivers were inconsistent. This tries to be more
explicit about what you are expected to do (bind, then release)
and matches the behavior of the Snowflake driver.
@lidavidm lidavidm marked this pull request as ready for review April 27, 2026 00:43
@lidavidm lidavidm merged commit d28465d into apache:main Apr 27, 2026
38 of 40 checks passed
@lidavidm lidavidm deleted the clarify-go-bind branch April 27, 2026 01:13
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.

2 participants