fix: handle driver.ErrSkip to avoid duplicate hooks execution with MySQL driver

[fangwentong]

Fix duplicate hooks execution with MySQL driver when InterpolateParams=false

Problem

When the MySQL driver is configured with InterpolateParams=false (default value), it returns driver.ErrSkip which causes the SQL package to fall back to using prepared statements. This results in hooks being executed twice for a single logical operation:

1. First time: Before + OnError (with driver.ErrSkip)
2. Second time: Before + After/OnError for the actual query execution

This double execution of hooks is unexpected and can lead to issues such as duplicate logging, metrics, or other side effects.

Solution

This change handles driver.ErrSkip internally by adding special handling in the hook execution flow, ensuring that hooks are only executed once per logical operation, even when the MySQL driver returns driver.ErrSkip.

Implementation

• Refactored code to extract execWithHooks and queryWithHooks helper functions for cleaner code organization
• Added special handling for driver.ErrSkip to prevent duplicate hook executions
• Added appropriate error handling to maintain compatibility with the SQL package's behavior

Test Changes

• Modified the test suite to count hook executions instead of simply checking if they ran
• Added assertions to verify hooks are executed exactly once per operation

Related Issues

This fixes a long-standing issue that has been reported multiple times:

• driver.ErrSkip error with sqlx #38
• how to log sql when sql execute failed? #32
• [Discussion] onError hook error ignore #52

[Copilot]

Pull Request Overview

This PR addresses the duplicate hook execution issue when using the MySQL driver with InterpolateParams=false by implementing special handling for driver.ErrSkip.

• Refactored hook execution functions to avoid duplicate invocation
• Updated tests to verify that each hook is executed exactly once per operation

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
sqlhooks_test.go	Updated tests to count hook executions instead of simple booleans.
sqlhooks.go	Refactored ExecContext and QueryContext to use helper functions that handle driver.ErrSkip.

[fangwentong]

@qustavo
I updated this PR and added the logic of closing prepared stmt, see the diff, referring to the implementation of golang sql package:

1. For QueryContext, delay the logic of stmt close to rows.Close()
2. For ExecContext, put the logic of stmt close after creating sql.Result.

[qustavo]

Thanks for your contribution!
Is there a case, apart from the one you describe above, where the MyQql driver returns driver.ErrSkip? More specifically, when you setup your connection with InterpolateParams=true, are there cases where we can still get that error; triggered by other situations?
If this is true, how does your fix handle those? Will it forward the driver.ErrSkip?

[fangwentong]

Thanks for your review!

After examining the MySQL driver code (*mysqlConn.interpolateParams), I can confirm that it returns driver.ErrSkip in several situations even with InterpolateParams=true:

1. When the number of placeholders (?) doesn't match the number of arguments
2. For unsupported argument types (types not explicitly handled in the switch statement)
3. When the resulting query would exceed the max allowed packet size

My fix handles all these cases:

• When driver.ErrSkip is returned (regardless of reason), we use prepared statements + Exec/Query as the fallback path. The result from stmt.Exec/stmt.Query is returned to sql package, which won't return driver.ErrSkip error
• Crucially, we avoid executing hooks twice by directly calling the internal prepare/execute methods
• This maintains the same behavior as the standard database/sql package for handling driver.ErrSkip returned from Exec/ExecContext/Query/QueryContext

To summarize, this change incorporates the sql package's handling of driver.ErrSkip directly into sqlhooks. After this modification, sqlhooks' Exec/ExecContext/Query/QueryContext won't return driver.ErrSkip, reducing the interaction between the sql package and sqlhooks to a single call, rather than multiple calls as before (first Exec/Query returning driver.ErrSkip, then a second Prepare+Query/Exec).

This approach is general and works for all driver.ErrSkip scenarios, not just the InterpolateParams=false case.