diff --git a/input/postgres/explain.go b/input/postgres/explain.go index 140ca8bbe..7f1718b7d 100644 --- a/input/postgres/explain.go +++ b/input/postgres/explain.go @@ -6,8 +6,6 @@ import ( "strings" "github.com/guregu/null" - pg_query "github.com/lfittl/pg_query_go" - pg_query_nodes "github.com/lfittl/pg_query_go/nodes" "github.com/lib/pq" "github.com/pganalyze/collector/output/pganalyze_collector" "github.com/pganalyze/collector/state" @@ -75,13 +73,8 @@ func runDbExplain(db *sql.DB, inputs []state.PostgresQuerySample, useHelper bool for _, sample := range inputs { // To be on the safe side never EXPLAIN a statement that can't be parsed, // or multiple statements in one (leading to accidental execution) - parsetree, err := pg_query.Parse(sample.Query) - if err != nil || len(parsetree.Statements) != 1 { - continue - } - stmt := parsetree.Statements[0].(pg_query_nodes.RawStmt).Stmt - switch stmt.(type) { - case pg_query_nodes.SelectStmt, pg_query_nodes.InsertStmt, pg_query_nodes.UpdateStmt, pg_query_nodes.DeleteStmt: + isUtil, err := util.IsUtilityStmt(sample.Query) + if err == nil && len(isUtil) == 1 && !isUtil[0] { sample.HasExplain = true sample.ExplainSource = pganalyze_collector.QuerySample_STATEMENT_LOG_EXPLAIN_SOURCE sample.ExplainFormat = pganalyze_collector.QuerySample_JSON_EXPLAIN_FORMAT diff --git a/input/postgres/statements.go b/input/postgres/statements.go index 20dc163d5..5e51506df 100644 --- a/input/postgres/statements.go +++ b/input/postgres/statements.go @@ -211,6 +211,10 @@ func GetStatements(server *state.Server, logger *util.Logger, db *sql.DB, global statements[key] = stmt } + if ignoreIOTiming(postgresVersion, receivedQuery) { + stats.BlkReadTime = 0 + stats.BlkWriteTime = 0 + } statementStats[key] = stats } err = rows.Err() @@ -224,3 +228,24 @@ func GetStatements(server *state.Server, logger *util.Logger, db *sql.DB, global return statements, statementTexts, statementStats, nil } + +func ignoreIOTiming(postgresVersion state.PostgresVersion, receivedQuery null.String) bool { + // Currently, Aurora gives wildly incorrect blk_read_time and blk_write_time values + // for utility statements; ignore I/O timing in this situation. + if !postgresVersion.IsAwsAurora || !receivedQuery.Valid { + return false + } + + isUtil, err := util.IsUtilityStmt(receivedQuery.String) + if err != nil { + return false + } + + for _, isOneUtil := range isUtil { + if isOneUtil { + return true + } + } + + return false +} diff --git a/util/is_utility.go b/util/is_utility.go new file mode 100644 index 000000000..34b4191ed --- /dev/null +++ b/util/is_utility.go @@ -0,0 +1,28 @@ +package util + +import ( + pg_query "github.com/lfittl/pg_query_go" + pg_query_nodes "github.com/lfittl/pg_query_go/nodes" +) + +// IsUtilityStmt determines whether each statement in the query text is a +// utility statement or a standard SELECT/INSERT/UPDATE/DELETE statement. +func IsUtilityStmt(query string) ([]bool, error) { + var result []bool + parsetree, err := pg_query.Parse(query) + if err != nil { + return nil, err + } + for _, rawStmt := range parsetree.Statements { + stmt := rawStmt.(pg_query_nodes.RawStmt).Stmt + var isUtility bool + switch stmt.(type) { + case pg_query_nodes.SelectStmt, pg_query_nodes.InsertStmt, pg_query_nodes.UpdateStmt, pg_query_nodes.DeleteStmt: + isUtility = false + default: + isUtility = true + } + result = append(result, isUtility) + } + return result, nil +} diff --git a/util/is_utility_test.go b/util/is_utility_test.go new file mode 100644 index 000000000..add8bdf48 --- /dev/null +++ b/util/is_utility_test.go @@ -0,0 +1,72 @@ +package util_test + +import ( + "reflect" + "testing" + + "github.com/pganalyze/collector/util" +) + +var isUtilTests = []struct { + input string + expected []bool + expectErr bool +}{ + { + "SELECT 1", + []bool{false}, + false, + }, + { + "INSERT INTO my_table VALUES(123)", + []bool{false}, + false, + }, + { + "UPDATE my_table SET foo = 123", + []bool{false}, + false, + }, + { + "DELETE FROM my_table", + []bool{false}, + false, + }, + { + "SHOW fsync", + []bool{true}, + false, + }, + { + "SET fsync = off", + []bool{true}, + false, + }, + { + "SELECT 1; SELECT 2;", + []bool{false, false}, + false, + }, + { + "SELECT 1; SHOW fsync;", + []bool{false, true}, + false, + }, + { + "totally not valid sql", + nil, + true, + }, +} + +func TestIsUtilityStmt(t *testing.T) { + for _, test := range isUtilTests { + actual, err := util.IsUtilityStmt(test.input) + if (err != nil) != test.expectErr { + t.Errorf("IsUtilityStmt(%s): expected err: %t; actual: %s", test.input, test.expectErr, err) + } + if !reflect.DeepEqual(actual, test.expected) { + t.Errorf("IsUtilityStmt(%s): expected %v; actual %v", test.input, test.expected, actual) + } + } +}