From cba610c245265ff50ea3c56a9961da218ed7d730 Mon Sep 17 00:00:00 2001 From: Jack Christensen Date: Wed, 11 Nov 2020 15:52:59 -0600 Subject: [PATCH] StatementErrored does not need context nor return an error --- stmtcache/lru.go | 14 ++++---------- stmtcache/lru_test.go | 7 +++---- stmtcache/stmtcache.go | 10 ++++------ 3 files changed, 11 insertions(+), 20 deletions(-) diff --git a/stmtcache/lru.go b/stmtcache/lru.go index 2f183f90..f58f2ac3 100644 --- a/stmtcache/lru.go +++ b/stmtcache/lru.go @@ -88,24 +88,18 @@ func (c *LRU) Clear(ctx context.Context) error { return nil } -func (c *LRU) StatementErrored(ctx context.Context, sql string, err error) error { +func (c *LRU) StatementErrored(sql string, err error) { pgErr, ok := err.(*pgconn.PgError) if !ok { - // we don't know how to handle this error - return nil + return } isInvalidCachedPlanError := pgErr.Severity == "ERROR" && pgErr.Code == "0A000" && pgErr.Message == "cached plan must not change result type" - if !isInvalidCachedPlanError { - // only flush if a plan has been changed out from under us - return nil + if isInvalidCachedPlanError { + c.stmtsToClear = append(c.stmtsToClear, sql) } - - c.stmtsToClear = append(c.stmtsToClear, sql) - - return nil } func (c *LRU) clearStmt(ctx context.Context, sql string) error { diff --git a/stmtcache/lru_test.go b/stmtcache/lru_test.go index 58a0c378..2d620905 100644 --- a/stmtcache/lru_test.go +++ b/stmtcache/lru_test.go @@ -89,8 +89,7 @@ func TestLRUStmtInvalidation(t *testing.T) { require.EqualValues(t, 1, cache.Len()) require.ElementsMatch(t, []string{"select 1"}, fetchServerStatements(t, ctx, conn)) - err = cache.StatementErrored(ctx, "select 1", fakeInvalidCachePlanError) - require.NoError(t, err) + cache.StatementErrored("select 1", fakeInvalidCachePlanError) _, err = cache.Get(ctx, "select 2") require.NoError(t, err) require.EqualValues(t, 1, cache.Len()) @@ -117,7 +116,7 @@ func TestLRUStmtInvalidation(t *testing.T) { require.Error(t, res.Close()) require.Equal(t, byte('E'), conn.TxStatus()) - err = cache.StatementErrored(ctx, "select 1", fakeInvalidCachePlanError) + cache.StatementErrored("select 1", fakeInvalidCachePlanError) require.EqualValues(t, 1, cache.Len()) res = conn.Exec(ctx, "rollback") @@ -156,7 +155,7 @@ func TestLRUStmtInvalidationIntegration(t *testing.T) { result = conn.ExecPrepared(ctx, sd1.Name, nil, nil, nil).Read() require.EqualError(t, result.Err, "ERROR: cached plan must not change result type (SQLSTATE 0A000)") - cache.StatementErrored(ctx, sql, result.Err) + cache.StatementErrored(sql, result.Err) sd2, err := cache.Get(ctx, sql) require.NoError(t, err) diff --git a/stmtcache/stmtcache.go b/stmtcache/stmtcache.go index 6e88ba54..d083e1b4 100644 --- a/stmtcache/stmtcache.go +++ b/stmtcache/stmtcache.go @@ -21,12 +21,10 @@ type Cache interface { Clear(ctx context.Context) error // StatementErrored informs the cache that the given statement resulted in an error when it - // was last used against the database. In some cases, this will cause the cache to flush - // the statement from the cache. It will only do so when the underlying `*pgconn.PgConn` - // is not currently in a transaction. If the connection is in the middle of a transaction, - // the bad statement will instead be flushed during the next call to Get that occurrs outside - // of a transaction. - StatementErrored(ctx context.Context, sql string, err error) error + // was last used against the database. In some cases, this will cause the cache to maer that + // statement as bad. The bad statement will instead be flushed during the next call to Get + // that occurs outside of a failed transaction. + StatementErrored(sql string, err error) // Len returns the number of cached prepared statement descriptions. Len() int