go-pg / pg

Golang ORM with focus on PostgreSQL features and performance
https://pg.uptrace.dev/
BSD 2-Clause "Simplified" License
5.67k stars 404 forks source link

CountEstimate is not safe to be called from transaction context #1945

Open Ezian opened 2 years ago

Ezian commented 2 years ago

Expected Behavior

CountEstimate(threshold int) should be safely called from any context, including transaction.

Current Behavior

If a transaction is run containing a CountEstimate() instruction without preliminary run of other query using this function, the transaction will fail.

It is due to the fact that CountEstimate() use an error-based lazy loading in count_estimate.go


if err != nil {
  if pgerr, ok := err.(internal.PGError); ok && pgerr.Field('C') == "42883" {
    // undefined_function
    err = q.createCountEstimateFunc()
    // ...
}

This lazy-loading cause a rollback of the transaction, which fails.

Possible Solution

Several possible solutions:

  1. Change the "lazy init" of this function by checking it's existence whenever we use the function CountEstimate()... But can lead to performance issue...
  2. Handle the specific case of transaction whenever we fails to create this function
  3. Don't create the pgCountEstimateFunc from the query db whenever it's a transaction, but from the baseDB of the transaction...
  4. Make it a feature, add some docs to the function with the workaround ^^

Steps to Reproduce

(Error handling is omitted)

func main(){
  dbh, _ := getDBHandler() // return a db handler (*pg.DB) on a "clean" db (without _go_pg_count_estimate_v2 function)

  tx, _ := db.Begin()
  _, err := tx.Model(&SomeModel{}).CountEstimate(10)
  if err != nil {
     return panic(fmt.Sprintf("countEstimate failure. %s", err))
  }
}

This code will panic with message: panic: countEstimate failure. ERROR #25P02 current transaction is aborted, commands ignored until end of transaction block

The code below will succeed, since the first query will create the _go_pg_count_estimate_v2, and avoid the error in the transaction

func main(){
  dbh, _ := getDBHandler() // return a db handler (*pg.DB) on a "clean" db (without _go_pg_count_estimate_v2 function)

  // This will create the _go_pg_count_estimate_v2 function, which will make it available in above transaction
  db.Model(&SomeModel{}).CountEstimate(10)

  tx, _ := db.Begin()
  _, err := tx.Model(&SomeModel{}).CountEstimate(10)
  if err != nil {
     return panic(fmt.Sprintf("countEstimate failure. %s", err))
  }
}

Context (Environment)