Make Encode return a result #3126

FSMaxB · 2024-03-16T11:44:00Z

~~Still working on this, but wanted to make a draft PR where I can push my current progress.~~

My approach was to incrementally turn return values into Result in one layer, but keeping the panic by calling .expect(...) on the layers that use those, then go to the next layer and turn the .expect(...) into ? and push the panics up further and further until they're gone.

I'll leave some open questions as review comments.

FSMaxB · 2024-03-17T10:53:59Z

Getting the macros to work again is really tricky as it turns out!

FSMaxB

Leaving some comments and open questions.

sqlx-core/src/any/arguments.rs

sqlx-core/src/any/connection/executor.rs

sqlx-core/src/executor.rs

sqlx-core/src/query.rs

sqlx-core/src/types/json.rs

sqlx-macros-core/src/query/args.rs

sqlx-mysql/src/connection/executor.rs

sqlx-postgres/src/types/interval.rs

sqlx-core/src/query.rs

src/lib.rs

sqlx-postgres/src/types/bigdecimal.rs

sqlx-mysql/src/types/json.rs

sqlx-mysql/src/arguments.rs

sqlx-postgres/src/arguments.rs

abonander

In general, we need to be careful not to leave argument buffers in an invalid state on error, in case someone decides to retry.

This only changes the trait for now and makes it compile, calling .expect() on all users. Those will be removed in a later commit.

This pushes the panics one level further to mostly bind calls. Those will also be removed later.

FSMaxB · 2024-04-26T16:35:06Z

@abonander I believe I have addressed all review comments, so this is ready for another round of review.

abonander

Looks good with one final nit.

sqlx-mysql/src/protocol/statement/execute.rs

sqlx-mysql/src/types/json.rs

* Make encode and encode_by_ref fallible This only changes the trait for now and makes it compile, calling .expect() on all users. Those will be removed in a later commit. * PgNumeric: Turn TryFrom Decimal to an infallible From * Turn panics in Encode implementations into errors * Add Encode error analogous to the Decode error * Propagate decode errors through Arguments::add This pushes the panics one level further to mostly bind calls. Those will also be removed later. * Only check argument encoding at the end * Use Result in Query internally * Implement query_with functions in terms of _with_result * Surface encode errors when executing a query. * Remove remaining panics in AnyConnectionBackend implementations * PostgreSQL BigDecimal: Return encode error immediately * Arguments: Add len method to report how many arguments were added * Query::bind: Report which argument failed to encode * IsNull: Add is_null method * MySqlArguments: Replace manual bitmap code with NullBitMap helper type * Roll back buffer in MySqlArguments if encoding fails * Roll back buffer in SqliteArguments if encoding fails * Roll back PgArgumentBuffer if encoding fails

FSMaxB force-pushed the fallible-encode branch 3 times, most recently from 9dde82f to 4a5a6bd Compare March 16, 2024 14:55

FSMaxB force-pushed the fallible-encode branch 7 times, most recently from e831255 to c9226b1 Compare April 10, 2024 11:06

FSMaxB marked this pull request as ready for review April 10, 2024 11:19

FSMaxB commented Apr 10, 2024

View reviewed changes