sql: add the ability to execute multiple statements in sql_raw plugins #3113
Conversation
| *Type*: `bool` | ||
|
|
||
|
|
||
| === `queries` |
There was a problem hiding this comment.
I am open to different designs/semantics.
@ligfx would love your thoughts feedback on this (there is an example above)
There was a problem hiding this comment.
Looks good to me 👍 This is pretty much the design I imagined when you said you wanted to have each query as a separate item.
There was a problem hiding this comment.
That is a good sign - really appreciate you taking a look!
| _, err := db.Exec(fmt.Sprintf(`create table %s ( | ||
| "foo" varchar(50) not null, | ||
| "bar" integer not null, | ||
| "bar" binary_float not null, |
There was a problem hiding this comment.
This fixed the oracle tests, which are failing in main.
| } | ||
|
|
||
| batch = batch.Copy() | ||
|
|
||
| for i, msg := range batch { |
There was a problem hiding this comment.
The logic here is a little more complex than I would like, so please do a close pass to make sure I didn't muck something up.
|
Will update the changelog prior to merging |
internal/impl/sql/output_sql_raw.go
Outdated
| if err != nil { | ||
| return err | ||
| msg.SetError(err) |
There was a problem hiding this comment.
SetError doesn't work for outputs, we need to nack failed messages. The lazy way is to return the error and fail the whole batch, which is fine for now as it's what we do currently. However, eventually we'd want to create a BatchError and break down the errors by index to make retries more efficient and targeted.
There was a problem hiding this comment.
Oof, duh. Thanks for catching. I rewrote this whole loop to use WalkWithBatchedErrors and it's better now.
internal/impl/sql/output_sql_raw.go
Outdated
| return err | ||
| if tx != nil { | ||
| if err != nil { | ||
| msg.SetError(err) |
There was a problem hiding this comment.
As above, we need to return the errors.
internal/impl/sql/output_sql_raw.go
Outdated
|
|
||
| if _, err := s.db.ExecContext(ctx, queryStr, args...); err != nil { | ||
| return err | ||
| if tx != nil { |
There was a problem hiding this comment.
We need a check for tx == nil that returns errors if appropriate.
| } else { | ||
| rows, err := s.db.QueryContext(ctx, queryStr, args...) | ||
| } | ||
| if tx != nil { |
There was a problem hiding this comment.
We need a tx == nil check on errors added as well.
A slightly different take on #2707 that doesn't require needing to parse SQL statements.