Merge #119981 #119986

119981: stmtdiagnostics: reduce row count in a test r=yuzefovich a=yuzefovich We can go from 10k to 100 for the large table while still getting the desired plans. Informs: #119945 Epic: None Release note: None 119986: sql: enhance error messages on validation of rows in virtual tables r=yuzefovich a=yuzefovich We just saw an internal error when populating `crdb_internal.cluster_locks` virtual table that was missing important bits (the column name) to understand what happens. Since names of virtual tables and columns in them don't contain PII, they are safe for redaction. Epic: None Release note: None Co-authored-by: Yahor Yuzefovich <[email protected]>
cockroachdb · Mar 6, 2024 · ce5f34e · ce5f34e
3 parents d400c11 + 97a0e35 + 22f66c6
commit ce5f34e
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 3 deletions.
diff --git a/pkg/sql/stmtdiagnostics/statement_diagnostics_test.go b/pkg/sql/stmtdiagnostics/statement_diagnostics_test.go
@@ -483,7 +483,7 @@ func TestDiagnosticsRequest(t *testing.T) {
 		runner.Exec(t, "ANALYZE small;")
 		runner.Exec(t, "CREATE TABLE large (v INT, INDEX (v));")
 		runner.Exec(t, "INSERT INTO large VALUES (1);")
-		runner.Exec(t, "INSERT INTO large SELECT 2 FROM generate_series(1, 10000);")
+		runner.Exec(t, "INSERT INTO large SELECT 2 FROM generate_series(1, 100);")
 		runner.Exec(t, "ANALYZE large;")
 
 		// query1 results in scan + lookup join whereas query2 does two scans +

diff --git a/pkg/sql/virtual_schema.go b/pkg/sql/virtual_schema.go
@@ -45,6 +45,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/iterutil"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/redact"
 )
 
 const virtualSchemaNotImplementedMessage = "virtual schema table not implemented: %s.%s"
@@ -566,15 +567,18 @@ func (e *virtualDefEntry) validateRow(datums tree.Datums, columns colinfo.Result
 	}
 	for i := range columns {
 		col := &columns[i]
+		// Names of virtual tables and columns in them don't contain any PII, so
+		// we can always mark them safe for redaction.
+		colName := redact.SafeString(col.Name)
 		datum := datums[i]
 		if datum == tree.DNull {
 			if !e.desc.PublicColumns()[i].IsNullable() {
 				return errors.AssertionFailedf("column %s.%s not nullable, but found NULL value",
-					e.desc.GetName(), col.Name)
+					redact.SafeString(e.desc.GetName()), colName)
 			}
 		} else if !datum.ResolvedType().Equivalent(col.Typ) {
 			return errors.AssertionFailedf("datum column %q expected to be type %s; found type %s",
-				col.Name, col.Typ, datum.ResolvedType())
+				colName, col.Typ.SQLStringForError(), datum.ResolvedType().SQLStringForError())
 		}
 	}
 	return nil