go-sql-spanner icon indicating copy to clipboard operation
go-sql-spanner copied to clipboard

ExecQuery with a DML statement with THEN RETURN in AutoCommit=true mode uses read-only transaction

Open olavloite opened this issue 1 year ago • 2 comments

Calling ExecQuery (or similar) with a DML statement that contains a THEN RETURN clause while the connection is in auto-commit mode, causes the driver to try to execute the DML statement using a read-only transaction.

olavloite avatar May 13 '24 13:05 olavloite

A simple fix could be to check whether the query string contains THEN RETURN, and if it does then execute it as a RW transaction. e.g.

var rxThenReturn = regexp.MustCompile(`(?i)\bTHEN[\s\r\n]+RETURN\b`)

// isProbablyReadWrite returns true when the query should use a ReadWrite transactions.
// It may return true in some corner cases, where it is a read-only query.
func isProbablyReadWrite(query string) (bool, error) {
       query, err := removeCommentsAndTrim(query)
       if err != nil {
               return false, err
       }

       return rxThenReturn.MatchString(query), nil
}

Executing a read-only statement in a read-write transaction shouldn't cause problems AFAIK.

egonelbre avatar Jul 12 '24 11:07 egonelbre

@egonelbre We'd rather not introduce more regex checking in this driver. I'm currently working on retrofitting some code from our Java ecosystem into this driver that should allow us to check for the existence of a THEN RETURN clause without the use of regexes. (It should also allow us to improve the current implementation for replacing positional parameters for named parameters by removing the step that first removes all comments from the SQL string).

olavloite avatar Jul 16 '24 06:07 olavloite