Skip to content

Conversation

@adhi-thirumala
Copy link

Description

In Row::get_value() and Row::get<T> for the Rust bindings, the underlying Vec that holds the queried values is accessed by [] such that out of bounds accesses cause a panic. Changed this to .get() on the Vec such that we can return an Err instead of panicking. Also added a test to verify this behavior (not sure if I had to, if this is unnecessary, I can remove).

Motivation and context

Issue #4339 said that this was behavior. Seemingly, current behavior is not in line with rustqlite::row::Row::get. It is also intuitively the case that methods should not panic on OOB access, especially not without documentation. Noticed that the same behavior happens in libsql::Row::get_value and libsql::Row::get<T>. I will submit same PR there as well.

Description of AI Usage

Claude code wrote the test.

Copy link

@turso-bot turso-bot bot left a comment

Choose a reason for hiding this comment

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

Please review @jussisaurio

@adhi-thirumala
Copy link
Author

is the bench test making a request that times out a known issue?

@jussisaurio
Copy link
Collaborator

is the bench test making a request that times out a known issue?

github has been really unreliable lately

@PThorpe92
Copy link
Collaborator

Will merge as soon as conflicts resolved 👍 looks good

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants