sqlx icon indicating copy to clipboard operation
sqlx copied to clipboard

NamedStmtContext - prepared statement doesn't support slice of struct?

Open kunwardeep opened this issue 6 years ago • 8 comments

I tried to do this

txn.NamedStmtContext(ctx, namedStatement).ExecContext(ctx, sqlStatement.Args)

Instead of executing I get a panic

panic: reflect: call of github.com/jmoiron/sqlx/reflectx.(*Mapper).TraversalsByNameFunc on slice Value [recovered]
     | 	panic: reflect: call of github.com/jmoiron/sqlx/reflectx.(*Mapper).TraversalsByNameFunc on slice Value
     | goroutine 72 [running]:
     | testing.tRunner.func1(0xc000350200)
     | 	/usr/local/Cellar/go/1.13.3/libexec/src/testing/testing.go:874 +0x69f
     | panic(0x197fba0, 0xc0001a2e80)
     | 	/usr/local/Cellar/go/1.13.3/libexec/src/runtime/panic.go:679 +0x1b2
     | github.com/jmoiron/sqlx/reflectx.mustBe(0x6c76630, 0x193cc60, 0x19)
     | 	/Users/<local_dir>/vendor/github.com/jmoiron/sqlx/reflectx/reflect.go:249 +0xfc
     | github.com/jmoiron/sqlx/reflectx.(*Mapper).TraversalsByNameFunc(0xc0001be1b0, 0x1b7fb40, 0x193cc60, 0xc0001cf7c0, 0x8, 0xa, 0xc000149660, 0x0, 0x1)
     | 	/Users/<local_dir>/vendor/github.com/jmoiron/sqlx/reflectx/reflect.go:187 +0xb3
     | github.com/jmoiron/sqlx.bindArgs(0xc0001cf7c0, 0x8, 0xa, 0x193cc60, 0xc0001a2d20, 0xc0001be1b0, 0x0, 0x2b846d0, 0x0, 0xc0001a2e60, ...)
     | 	/Users/<local_dir>/vendor/github.com/jmoiron/sqlx/named.go:168 +0x20c
     | github.com/jmoiron/sqlx.bindAnyArgs(0xc0001cf7c0, 0x8, 0xa, 0x193cc60, 0xc0001a2d20, 0xc0001be1b0, 0xc0000ca008, 0xc0001c0990, 0xc0001c0ab0, 0x0, ...)
     | 	/Users/<local_dir>/vendor/github.com/jmoiron/sqlx/named.go:153 +0x168
     | github.com/jmoiron/sqlx.(*NamedStmt).ExecContext(0xc000149908, 0x1b70cc0, 0xc0000ca008, 0x193cc60, 0xc0001a2d20, 0xc0001a2e60, 0xc0001a2e40, 0x0, 0x0)
     | 	/Users/<local_dir>/vendor/github.com/jmoiron/sqlx/named_context.go:37 +0x10c
     

To reproduce

args = map[string]interface{}{"id": 1}
deleteQry := delete from table_name where id =: id
deleteStatement, err := sqlx..PrepareNamed(deleteQry)
if err != nil {
	return nil, err
}
txn, errBeginTransaction:= sqlx.BeginTxx(ctx, &sql.TxOptions{Isolation: isolationLevel})
txn.NamedStmtContext(ctx, deleteStatement).ExecContext(ctx, args)

It might be related to this https://github.com/jmoiron/sqlx/issues/519

kunwardeep avatar Dec 03 '19 01:12 kunwardeep

See #285 (which is not yet in a release version).

dcormier avatar Dec 04 '19 15:12 dcormier

It does work with named queries(as we are using the latest master) but unfortunately, we need to use named queries in our transactions.

So when you do this to build a named query

	namedQry, err := dbClient.PrepareNamed(qryString)
	if err != nil {
		return nil, err
	}

In order to use namedQry we would need to do something like this -

txn.NamedStmtContext(ctx, namedQry).ExecContext(ctx, args)

The line fails with the error as mentioned above. It works perfectly well if I use query string directly in the transaction, but not named query

kunwardeep avatar Dec 04 '19 23:12 kunwardeep

I'm using (*sqlx.Tx).NamedExecContext and passing a slice of structs without issue, but, obviously, that's not a prepared statment.

Looking at the code in that vs (*NamesStmt).ExecContext, it does appear that a NamedStmt will not properly bind slice args. Interesting. Internally, (*sqlx.Tx).NamedExecContext calls bindNamedMapper while (*sqlx.NamedStmt).ExecContext calls bindAnyArgs.

I think you might be able to work around this by doing something similar to this:

query, mappedArgs, err := db.BindNamed(query, args)
if err != nil {
    panic(err)
}

nq, err := db.PrepareNamed(query)
if err != nil {
    panic(err)
}

tx, err := db.BeginTxx(ctx, nil)
if err != nil {
    panic(err)
}

tx.NamedStmtContext(ctx, nq).ExecContext(ctx, mappedArgs)

The call to (*sqlx.DB).BindNamed explicitly passes your args through the internal bindNamedArgs function.

I haven't tested this, but it looks like it should be a workaround.

All that said, it does seem like methods on NamedStmt should be expected to handle slice args in the same way that NamedExectContext does. I don't know if that was an oversight in #285, or if it's intentional. I might see about opening a PR for it.

dcormier avatar Dec 05 '19 14:12 dcormier

~I think I'm not far from a fix for this.~


Edit: I'm actually not sure how this can work the way you're trying to use it. Maybe someone else will come along and point out how this isn't the case.

Here's the thing: when using sqlx to execute a statement that takes a variable number of arguments (like when you have a slice or array of values to use as arguments), the SQL that needs to be prepared would depend on the arguments passed in. This is because sqlx uses those arguments to generate the complete SQL statement with all the correct parameters.

Using (*sqlx.DB).BindNamed, (*sqlx.Tx).BindNamed, or sqlx.Named generates the correct SQL based on the arguments (including slices/arrays). The returned query can then be prepared and executed using the returned arguments. This appears to be the right way to do it.


Having said all that, your specific, original example it looks like you're trying to be able to delete some records based on IDs, right? You can still do bulk deletes (or other actions) based on things like IDs by using an IN clause. Here's how you can do that with sqlx like this:

ids := []int{1, 2, 3}
deleteQry, args, err := sqlx.In("delete from table_name where id in (?);", ids...)
if err != nil {
	panic(err)
}

deleteQry = db.Rebind(deleteQry)

deleteStatement, err := sqlx.PrepareNamed(deleteQry)
if err != nil {
	return nil, err
}

txn, errBeginTransaction:= sqlx.BeginTxx(ctx, &sql.TxOptions{Isolation: isolationLevel})
txn.NamedStmtContext(ctx, deleteStatement).ExecContext(ctx, args)

dcormier avatar Dec 05 '19 17:12 dcormier

Is there an update on this issue?

blackout1208 avatar May 02 '20 23:05 blackout1208

I don't think it's doable because of the way prepared statements work (an SQL limitation, not a sqlx limitation). See my previous comment.

dcormier avatar May 03 '20 17:05 dcormier

@dcormier Heh, good explanation. It's funny how the brain tries to rationalize the need for this kind of functionality.

The example we're imagining would only make sense when doing batches-of-batches, where each batch would be required to be the same size. Example: imagine you have a prepared statement capable of inserting a batch of 1000 rows at a time, and then go to insert 5003 rows. You could reuse the prepared statement 5 times; but then you're left with 3 leftover rows that cannot fit a prepared statement expecting 1000 rows' worth of arguments… and so you'd have to run a separate statement anyway.

Anyone doing absurd numbers of batches-of-batches may as well just use db.NamedExec(string, []map[string]interface{}) with the slice and no prepared query. If you're really performing that much work, executing a new (unprepared) statement once for every large batch is not likely to be the bottleneck. Save worrying about the use of prepared statements for single queries that cannot be batched.

frickenate avatar Apr 15 '21 03:04 frickenate

For the record: I also encountered this problem, but instead of calling func (*DB) PrepareNamedContext) and then executing that stmt, I just call func (*DB) NamedExecContext directly.

bartekpacia avatar Aug 11 '24 17:08 bartekpacia