PMM-12151-12273 Explain, table improvements. (#2349)

* PMM-12151-12273 Explain. * PMM-12151-12273 Fix. * PMM12151-12273 Helpers. * PMM-12151-12273 Fix comments. * PMM-12151-12273 Remove semicolons. * Update api/qanpb/object_details.proto Co-authored-by: Alex Tymchuk <[email protected]> * Update api/qanpb/object_details.proto Co-authored-by: Alex Tymchuk <[email protected]> * PMM-12151-12273 Regen. * Update qan-api2/models/metrics.go Co-authored-by: Artem Gavrilov <[email protected]> * Update qan-api2/models/metrics.go Co-authored-by: Artem Gavrilov <[email protected]> * Update agent/runner/actions/query_transform_test.go Co-authored-by: Artem Gavrilov <[email protected]> * Update agent/runner/actions/query_transform_test.go Co-authored-by: Artem Gavrilov <[email protected]> * PMM-12151-12273 Required changes. * PMM-23252-12273 Parallel. * PMM-12151-12273 Typo. * PMM-12151-12273 Gen. * PMM-12151-12273 Regen with newer mockery. * PMM-12151-12273 Add warning for trimmed queries. * PMM-12151-12273 Add test for trimmed queries. * PMM-12151-12273 Use example for explain in case of enabled examples. * PMM-12151-12273 Use same query source table even for Oracle MySQL >=8 --------- Co-authored-by: Alex Tymchuk <[email protected]> Co-authored-by: Nurlan Moldomurov <[email protected]> Co-authored-by: Artem Gavrilov <[email protected]>
percona · Jul 28, 2023 · 146167a · 146167a
1 parent 5a88533
commit 146167a
Show file tree

Hide file tree

Showing 24 changed files with 2,275 additions and 897 deletions.
diff --git a/agent/agents/mysql/perfschema/history.go b/agent/agents/mysql/perfschema/history.go
@@ -53,16 +53,6 @@ func getHistory(q *reform.Querier) (historyMap, error) {
 	return getHistoryRows(rows, q)
 }
 
-func getHistory80(q *reform.Querier) (historyMap, error) {
-	rows, err := q.SelectRows(eventsStatementsSummaryByDigestExamplesView, "WHERE DIGEST IS NOT NULL AND QUERY_SAMPLE_TEXT IS NOT NULL")
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to query events_statements_summary_by_digest")
-	}
-	defer rows.Close() //nolint:errcheck
-
-	return getHistoryRows(rows, q)
-}
-
 func getHistoryRows(rows *sql.Rows, q *reform.Querier) (historyMap, error) {
 	var err error
 	res := make(historyMap)

diff --git a/agent/agents/mysql/perfschema/perfschema.go b/agent/agents/mysql/perfschema/perfschema.go
@@ -313,16 +313,7 @@ func (m *PerfSchema) runHistoryCacheRefresher(ctx context.Context) {
 }
 
 func (m *PerfSchema) refreshHistoryCache() error {
-	mysqlVer := m.mySQLVersion()
-
-	var err error
-	var current historyMap
-	switch {
-	case mysqlVer.version >= 8 && mysqlVer.vendor == "oracle":
-		current, err = getHistory80(m.q)
-	default:
-		current, err = getHistory(m.q)
-	}
+	current, err := getHistory(m.q)
 	if err != nil {
 		return err
 	}

diff --git a/agent/runner/actions/mysql_explain_action.go b/agent/runner/actions/mysql_explain_action.go
@@ -83,6 +83,11 @@ func (a *mysqlExplainAction) Run(ctx context.Context) ([]byte, error) {
 		return nil, errors.New("Query to EXPLAIN is empty")
 	}
 
+	// You cant run Explain on trimmed queries.
+	if strings.HasSuffix(a.params.Query, "...") {
+		return nil, errors.New("EXPLAIN failed because the query was too long and trimmed. Set max-query-length to a larger value.")
+	}
+
 	// Explain is supported only for DML queries.
 	// https://dev.mysql.com/doc/refman/8.0/en/using-explain.html
 	if !isDMLQuery(a.params.Query) {
@@ -114,6 +119,13 @@ func (a *mysqlExplainAction) Run(ctx context.Context) ([]byte, error) {
 		IsDMLQuery: changedToSelect,
 	}
 
+	if a.params.Schema != "" {
+		_, err = tx.ExecContext(ctx, fmt.Sprintf("USE %#q", a.params.Schema))
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	switch a.params.OutputFormat {
 	case agentpb.MysqlExplainOutputFormat_MYSQL_EXPLAIN_OUTPUT_FORMAT_DEFAULT:
 		response.ExplainResult, err = a.explainDefault(ctx, tx)

diff --git a/agent/runner/actions/mysql_explain_action_test.go b/agent/runner/actions/mysql_explain_action_test.go
@@ -197,6 +197,22 @@ func TestMySQLExplain(t *testing.T) {
 		assert.Equal(t, er.Query, `SELECT * FROM city  WHERE Name='Rosario'`)
 	})
 
+	t.Run("Query longer than max-query-length", func(t *testing.T) {
+		t.Parallel()
+
+		params := &agentpb.StartActionRequest_MySQLExplainParams{
+			Dsn:          dsn,
+			Query:        `INSERT INTO city (Name)...`,
+			OutputFormat: agentpb.MysqlExplainOutputFormat_MYSQL_EXPLAIN_OUTPUT_FORMAT_DEFAULT,
+		}
+		a := NewMySQLExplainAction("", time.Second, params)
+		ctx, cancel := context.WithTimeout(context.Background(), a.Timeout())
+		defer cancel()
+
+		_, err := a.Run(ctx)
+		require.Error(t, err, "EXPLAIN failed because the query was too long and trimmed. Set max-query-length to a larger value.")
+	})
+
 	t.Run("LittleBobbyTables", func(t *testing.T) {
 		t.Parallel()
 

diff --git a/agent/runner/actions/query_transform.go b/agent/runner/actions/query_transform.go
@@ -23,6 +23,7 @@ import (
 //nolint:lll
 var (
 	dmlVerbs         = []string{"select", "insert", "update", "delete", "replace"}
+	commentsRe       = regexp.MustCompile(`(?s)\/\*(.*?)\*\/`)
 	selectRe         = regexp.MustCompile(`(?i)^select\s+(.*?)\bfrom\s+(.*?)$`)
 	updateRe         = regexp.MustCompile(`(?i)^update\s+(?:low_priority|ignore)?\s*(.*?)\s+set\s+(.*?)(?:\s+where\s+(.*?))?(?:\s+limit\s*[0-9]+(?:\s*,\s*[0-9]+)?)?$`)
 	deleteRe         = regexp.MustCompile(`(?i)^delete\s+(.*?)\bfrom\s+(.*?)$`)
@@ -31,8 +32,16 @@ var (
 	insertSetRe      = regexp.MustCompile(`(?i)(?:insert(?:\s+ignore)?|replace)\s+(?:.*?\binto)\s+(.*?)\s*set\s+(.*?)\s*(?:\blimit\b|on\s+duplicate\s+key.*)?\s*$`)
 )
 
+func prepareQuery(query string) string {
+	query = commentsRe.ReplaceAllString(query, "")
+	query = strings.ReplaceAll(query, "\t", " ")
+	query = strings.ReplaceAll(query, "\n", " ")
+	query = strings.TrimRight(query, ";")
+	return strings.TrimLeft(query, " ")
+}
+
 func isDMLQuery(query string) bool {
-	query = strings.ToLower(strings.TrimSpace(query))
+	query = strings.ToLower(prepareQuery(query))
 	for _, verb := range dmlVerbs {
 		if strings.HasPrefix(query, verb) {
 			return true
@@ -51,7 +60,7 @@ it able to explain DML queries on older MySQL versions and for unprivileged user
 
 // dmlToSelect returns query converted to select and boolean, if conversion were needed.
 func dmlToSelect(query string) (string, bool) {
-	query = strings.ReplaceAll(query, "\n", " ")
+	query = prepareQuery(query)
 
 	m := selectRe.FindStringSubmatch(query)
 	if len(m) > 1 {

diff --git a/agent/runner/actions/query_transform_test.go b/agent/runner/actions/query_transform_test.go
@@ -15,79 +15,163 @@
 package actions
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 )
 
-func TestDMLToSelect(t *testing.T) {
-	q, c := dmlToSelect(`SELECT nombre FROM tabla WHERE id = 0`)
-	assert.False(t, c)
-	assert.Equal(t, `SELECT nombre FROM tabla WHERE id = 0`, q)
-
-	q, c = dmlToSelect(`update ignore tabla set nombre = "carlos" where id = 0 limit 2`)
-	assert.True(t, c)
-	assert.Equal(t, `SELECT nombre = "carlos" FROM tabla WHERE id = 0`, q)
-
-	q, c = dmlToSelect(`update ignore tabla set nombre = "carlos" where id = 0`)
-	assert.True(t, c)
-	assert.Equal(t, `SELECT nombre = "carlos" FROM tabla WHERE id = 0`, q)
-
-	q, c = dmlToSelect(`update ignore tabla set nombre = "carlos" limit 1`)
-	assert.True(t, c)
-	assert.Equal(t, `SELECT nombre = "carlos" FROM tabla`, q)
-
-	q, c = dmlToSelect(`update tabla set nombre = "carlos" where id = 0 limit 2`)
-	assert.True(t, c)
-	assert.Equal(t, `SELECT nombre = "carlos" FROM tabla WHERE id = 0`, q)
-
-	q, c = dmlToSelect(`update tabla set nombre = "carlos" where id = 0`)
-	assert.True(t, c)
-	assert.Equal(t, `SELECT nombre = "carlos" FROM tabla WHERE id = 0`, q)
-
-	q, c = dmlToSelect(`update tabla set nombre = "carlos" limit 1`)
-	assert.True(t, c)
-	assert.Equal(t, `SELECT nombre = "carlos" FROM tabla`, q)
-
-	q, c = dmlToSelect(`delete from tabla`)
-	assert.True(t, c)
-	assert.Equal(t, `SELECT * FROM tabla`, q)
-
-	q, c = dmlToSelect(`delete from tabla join tabla2 on tabla.id = tabla2.tabla2_id`)
-	assert.True(t, c)
-	assert.Equal(t, `SELECT 1 FROM tabla join tabla2 on tabla.id = tabla2.tabla2_id`, q)
-
-	q, c = dmlToSelect(`insert into tabla (f1, f2, f3) values (1,2,3)`)
-	assert.True(t, c)
-	assert.Equal(t, `SELECT * FROM tabla  WHERE f1=1 and f2=2 and f3=3`, q)
-
-	q, c = dmlToSelect(`insert into tabla (f1, f2, f3) values (1,2)`)
-	assert.True(t, c)
-	assert.Equal(t, `SELECT * FROM tabla  LIMIT 1`, q)
-
-	q, c = dmlToSelect(`insert into tabla set f1="A1", f2="A2"`)
-	assert.True(t, c)
-	assert.Equal(t, `SELECT * FROM tabla WHERE f1="A1" AND  f2="A2"`, q)
-
-	q, c = dmlToSelect(`replace into tabla set f1="A1", f2="A2"`)
-	assert.True(t, c)
-	assert.Equal(t, `SELECT * FROM tabla WHERE f1="A1" AND  f2="A2"`, q)
-
-	q, c = dmlToSelect("insert into `tabla-1` values(12)")
-	assert.True(t, c)
-	assert.Equal(t, "SELECT * FROM `tabla-1` LIMIT 1", q)
-
-	q, c = dmlToSelect(`UPDATE
-  employees2
-SET
-  first_name = 'Joe',
-  emp_no = 10
-WHERE
-  emp_no = 3`)
-	assert.True(t, c)
-	assert.Equal(t, "SELECT first_name = 'Joe',   emp_no = 10 FROM employees2 WHERE emp_no = 3", q)
+func Test_dmlToSelect(t *testing.T) {
+	t.Parallel()
+
+	type testCase struct {
+		Query     string
+		Converted bool
+		Expected  string
+	}
+
+	testCases := []testCase{
+		{
+			Query:     `SELECT nombre FROM tabla WHERE id = 0`,
+			Converted: false,
+			Expected:  `SELECT nombre FROM tabla WHERE id = 0`,
+		},
+		{
+			Query:     `update ignore tabla set nombre = "carlos" where id = 0 limit 2`,
+			Converted: true,
+			Expected:  `SELECT nombre = "carlos" FROM tabla WHERE id = 0`,
+		},
+		{
+			Query:     `update ignore tabla set nombre = "carlos" where id = 0`,
+			Converted: true,
+			Expected:  `SELECT nombre = "carlos" FROM tabla WHERE id = 0`,
+		},
+		{
+			Query:     `update ignore tabla set nombre = "carlos" limit 1`,
+			Converted: true,
+			Expected:  `SELECT nombre = "carlos" FROM tabla`,
+		},
+		{
+			Query:     `update tabla set nombre = "carlos" where id = 0 limit 2`,
+			Converted: true,
+			Expected:  `SELECT nombre = "carlos" FROM tabla WHERE id = 0`,
+		},
+		{
+			Query:     `update tabla set nombre = "carlos" where id = 0`,
+			Converted: true,
+			Expected:  `SELECT nombre = "carlos" FROM tabla WHERE id = 0`,
+		},
+		{
+			Query:     `update tabla set nombre = "carlos" limit 1`,
+			Converted: true,
+			Expected:  `SELECT nombre = "carlos" FROM tabla`,
+		},
+		{
+			Query:     `delete from tabla`,
+			Converted: true,
+			Expected:  `SELECT * FROM tabla`,
+		},
+		{
+			Query:     `delete from tabla join tabla2 on tabla.id = tabla2.tabla2_id`,
+			Converted: true,
+			Expected:  `SELECT 1 FROM tabla join tabla2 on tabla.id = tabla2.tabla2_id`,
+		},
+		{
+			Query:     `insert into tabla (f1, f2, f3) values (1,2,3)`,
+			Converted: true,
+			Expected:  `SELECT * FROM tabla  WHERE f1=1 and f2=2 and f3=3`,
+		},
+		{
+			Query:     `insert into tabla (f1, f2, f3) values (1,2)`,
+			Converted: true,
+			Expected:  `SELECT * FROM tabla  LIMIT 1`,
+		},
+		{
+			Query:     `insert into tabla set f1="A1", f2="A2"`,
+			Converted: true,
+			Expected:  `SELECT * FROM tabla WHERE f1="A1" AND  f2="A2"`,
+		},
+		{
+			Query:     "insert into `tabla-1` values(12)",
+			Converted: true,
+			Expected:  "SELECT * FROM `tabla-1` LIMIT 1",
+		},
+		{
+			Query: `UPDATE
+				employees2
+			SET
+				first_name = 'Joe',
+				emp_no = 10
+			WHERE
+				emp_no = 3`,
+			Converted: true,
+			Expected:  `SELECT first_name = 'Joe',     emp_no = 10 FROM employees2 WHERE emp_no = 3`,
+		},
+		{
+			Query: `
+			/* File:movie.php Line:8 Func:update_info */
+				   SELECT
+				*
+			FROM
+				movie_info
+			WHERE
+				movie_id = 68357`,
+			Converted: false,
+			Expected:  `SELECT     *    FROM     movie_info    WHERE     movie_id = 68357`,
+		},
+		{
+			Query: `SELECT /*+ NO_RANGE_OPTIMIZATION(t3 PRIMARY, f2_idx) */ f1
+			FROM t3 WHERE f1 > 30 AND f1 < 33;`,
+			Converted: false,
+			Expected:  `SELECT  f1    FROM t3 WHERE f1 > 30 AND f1 < 33`,
+		},
+		{
+			Query:     `SELECT /*+ BKA(t1) NO_BKA(t2) */ * FROM t1 INNER JOIN t2 WHERE ...;`,
+			Converted: false,
+			Expected:  `SELECT  * FROM t1 INNER JOIN t2 WHERE ...`,
+		},
+		{
+			Query:     `SELECT /*+ NO_ICP(t1, t2) */ * FROM t1 INNER JOIN t2 WHERE ...;`,
+			Converted: false,
+			Expected:  `SELECT  * FROM t1 INNER JOIN t2 WHERE ...`,
+		},
+		{
+			Query:     `SELECT /*+ SEMIJOIN(FIRSTMATCH, LOOSESCAN) */ * FROM t1 ...;`,
+			Converted: false,
+			Expected:  `SELECT  * FROM t1 ...`,
+		},
+		{
+			Query:     `EXPLAIN SELECT /*+ NO_ICP(t1) */ * FROM t1 WHERE ...;`,
+			Converted: false,
+			Expected:  ``,
+		},
+		{
+			Query:     `SELECT /*+ MERGE(dt) */ * FROM (SELECT * FROM t1) AS dt;`,
+			Converted: false,
+			Expected:  `SELECT  * FROM (SELECT * FROM t1) AS dt`,
+		},
+		{
+			Query:     `INSERT /*+ SET_VAR(foreign_key_checks=OFF) */ INTO t2 VALUES(2);`,
+			Converted: true,
+			Expected:  `SELECT * FROM t2 LIMIT 1`,
+		},
+	}
+
+	for i, tc := range testCases {
+		tc := tc
+		t.Run(fmt.Sprintf("TestDMLToSelect %d. %s", i, tc.Query), func(t *testing.T) {
+			t.Parallel()
+			q, c := dmlToSelect(tc.Query)
+			assert.Equal(t, tc.Converted, c)
+			assert.Equal(t, tc.Expected, q)
+		})
+	}
+}
 
-	q, c = dmlToSelect(`UPDATE employees2 SET first_name = 'Joe', emp_no = 10 WHERE emp_no = 3`)
-	assert.True(t, c)
-	assert.Equal(t, "SELECT first_name = 'Joe', emp_no = 10 FROM employees2 WHERE emp_no = 3", q)
+func Test_isDMLQuery(t *testing.T) {
+	assert.True(t, isDMLQuery("SELECT * FROM table"))
+	assert.True(t, isDMLQuery(`update tabla set nombre = "carlos" where id = 0`))
+	assert.True(t, isDMLQuery("delete from tabla join tabla2 on tabla.id = tabla2.tabla2_id"))
+	assert.True(t, isDMLQuery("/*+ SET_VAR(foreign_key_checks=OFF) */ INSERT INTO t2 VALUES(2);"))
+	assert.False(t, isDMLQuery("EXPLAIN SELECT * FROM table"))
 }