Merge pull request vitessio#48 from planetscale/remove-duplicated-errors

Group same failed queries together in keys's output

Author: systay

go/keys/keys.go

@@ -48,6 +48,7 @@ func run(out io.Writer, cfg Config) error {
 	}
 	ql := &queryList{
 		queries: make(map[string]*QueryAnalysisResult),
+		failed:  make(map[string]*QueryFailedResult),
 	}
 	queries, err := cfg.Loader.Load(cfg.FileName)
 	if err != nil {
@@ -78,11 +79,7 @@ func run(out io.Writer, cfg Config) error {
 func process(q data.Query, si *schemaInfo, ql *queryList) {
 	ast, bv, err := sqlparser.NewTestParser().Parse2(q.Query)
 	if err != nil {
-		ql.failed = append(ql.failed, QueryFailedResult{
-			Query:      q.Query,
-			LineNumber: q.Line,
-			Error:      err.Error(),
-		})
+		ql.addFailedQuery(q, err)
 		return
 	}
 
@@ -102,40 +99,28 @@ type Output struct {
 
 type queryList struct {
 	queries map[string]*QueryAnalysisResult
-	failed  []QueryFailedResult
+	failed  map[string]*QueryFailedResult
 }
 
 func (ql *queryList) processQuery(si *schemaInfo, ast sqlparser.Statement, q data.Query, bv sqlparser.BindVars) {
 	// handle panics
 	defer func() {
 		if r := recover(); r != nil {
-			ql.failed = append(ql.failed, QueryFailedResult{
-				Query:      q.Query,
-				LineNumber: q.Line,
-				Error:      fmt.Sprintf("panic: %v", r),
-			})
+			ql.addFailedQuery(q, fmt.Errorf("panic: %v", r))
 		}
 	}()
 
 	mapBv := make(map[string]*querypb.BindVariable)
 	reservedVars := sqlparser.NewReservedVars("", bv)
 	err := sqlparser.Normalize(ast, reservedVars, mapBv)
 	if err != nil {
-		ql.failed = append(ql.failed, QueryFailedResult{
-			Query:      q.Query,
-			LineNumber: q.Line,
-			Error:      err.Error(),
-		})
+		ql.addFailedQuery(q, err)
 		return
 	}
 
 	st, err := semantics.Analyze(ast, "ks", si)
 	if err != nil {
-		ql.failed = append(ql.failed, QueryFailedResult{
-			Query:      q.Query,
-			LineNumber: q.Line,
-			Error:      err.Error(),
-		})
+		ql.addFailedQuery(q, err)
 		return
 	}
 	ctx := &plancontext.PlanningContext{
@@ -173,20 +158,40 @@ func (ql *queryList) processQuery(si *schemaInfo, ast sqlparser.Statement, q dat
 	}
 }
 
+func (ql *queryList) addFailedQuery(q data.Query, err error) {
+	key := q.Query + err.Error()
+	if v, exists := ql.failed[key]; exists {
+		v.LineNumbers = append(v.LineNumbers, q.Line)
+	} else {
+		ql.failed[key] = &QueryFailedResult{
+			Query:       q.Query,
+			LineNumbers: []int{q.Line},
+			Error:       err.Error(),
+		}
+	}
+}
+
 // writeJsonTo writes the query list, sorted by the first line number of the query, to the given writer.
 func (ql *queryList) writeJSONTo(w io.Writer) error {
 	values := make([]QueryAnalysisResult, 0, len(ql.queries))
 	for _, result := range ql.queries {
 		values = append(values, *result)
 	}
-
 	sort.Slice(values, func(i, j int) bool {
 		return values[i].LineNumbers[0] < values[j].LineNumbers[0]
 	})
 
+	failedQueries := make([]QueryFailedResult, 0, len(ql.failed))
+	for _, result := range ql.failed {
+		failedQueries = append(failedQueries, *result)
+	}
+	sort.Slice(failedQueries, func(i, j int) bool {
+		return failedQueries[i].LineNumbers[0] < failedQueries[j].LineNumbers[0]
+	})
+
 	res := Output{
 		Queries: values,
-		Failed:  ql.failed,
+		Failed:  failedQueries,
 	}
 
 	jsonData, err := json.MarshalIndent(res, "  ", "  ")
@@ -216,7 +221,7 @@ type QueryAnalysisResult struct {
 }
 
 type QueryFailedResult struct {
-	Query      string `json:"query"`
-	LineNumber int    `json:"lineNumber"`
-	Error      string `json:"error"`
+	Query       string `json:"query"`
+	LineNumbers []int  `json:"lineNumbers"`
+	Error       string `json:"error"`
 }

go/keys/keys_test.go

@@ -21,10 +21,10 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/vitessio/vt/go/data"
 	"github.com/vitessio/vt/go/typ"
-
-	"github.com/stretchr/testify/require"
 )
 
 func TestKeys(t *testing.T) {
@@ -50,6 +50,7 @@ func TestKeysNonAuthoritativeTable(t *testing.T) {
 	si := &schemaInfo{}
 	ql := &queryList{
 		queries: make(map[string]*QueryAnalysisResult),
+		failed:  make(map[string]*QueryFailedResult),
 	}
 	process(q, si, ql)
 

go/summarize/summarize-keys.go

@@ -58,6 +58,7 @@ type (
 	FailuresSummary struct {
 		Query string
 		Error string
+		Count int
 	}
 
 	graphKey struct {
@@ -296,10 +297,10 @@ func renderFailures(md *markdown.MarkDown, failures []FailuresSummary) {
 	}
 	md.PrintHeader("Failures", 2)
 
-	headers := []string{"Query", "Error"}
+	headers := []string{"Query", "Error", "Count"}
 	var rows [][]string
 	for _, failure := range failures {
-		rows = append(rows, []string{failure.Query, failure.Error})
+		rows = append(rows, []string{failure.Query, failure.Error, strconv.Itoa(failure.Count)})
 	}
 	md.PrintTable(headers, rows)
 }
@@ -368,6 +369,7 @@ func summarizeKeysQueries(queries *keys.Output) ([]TableSummary, []FailuresSumma
 		failures = append(failures, FailuresSummary{
 			Query: query.Query,
 			Error: query.Error,
+			Count: len(query.LineNumbers),
 		})
 	}
 

go/summarize/testdata/keys-log.json

@@ -525,12 +525,10 @@
     "failed": [
       {
         "query": "I am a failing query;",
-        "lineNumber": 463,
-        "error": "syntax error at position 2 near 'I'"
-      },
-      {
-        "query": "I am a failing query;",
-        "lineNumber": 781,
+        "lineNumbers": [
+          463,
+          781
+        ],
         "error": "syntax error at position 2 near 'I'"
       }
     ]

go/summarize/testdata/keys-summary.md

@@ -148,8 +148,7 @@ partsupp ↔ supplier (Occurrences: 1)
 
 ```
 ## Failures
-|Query|Error|
-|---|---|
-|I am a failing query;|syntax error at position 2 near 'I'|
-|I am a failing query;|syntax error at position 2 near 'I'|
+|Query|Error|Count|
+|---|---|---|
+|I am a failing query;|syntax error at position 2 near 'I'|2|