From b3bef41e89ef6af5858d07bb6af8407002a9d3c9 Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Tue, 29 Mar 2022 18:33:14 -0400 Subject: [PATCH] sql: ignore non-existent columns when injecting stats Previously, an `ALTER TABLE ... INJECT STATS` command would return an error if the given stats JSON included any columns that were not present in the table descriptor. Statistics in statement bundles often include dropped columns, so reproducing issues with a bundle required tediously removing stats for these columns. This commit changes the stats injection behavior so that a notice is issued for stats with non-existent columns rather than an error. Any stats for existing columns will be injected successfully. Informs #68184 Release note (sql change): `ALTER TABLE ... INJECT STATISTICS ...` will now issue notices when the given statistics JSON includes non-existent columns, rather than resulting in an error. Any statistics in the JSON for existing columns will be injected successfully. --- pkg/sql/alter_table.go | 8 ++++- .../testdata/logic_test/distsql_stats | 35 ++++++++++++++++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go index 0a9c37ca1aa2..dbb2f0e3971c 100644 --- a/pkg/sql/alter_table.go +++ b/pkg/sql/alter_table.go @@ -34,6 +34,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/parser" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice" "github.com/cockroachdb/cockroach/pkg/sql/privilege" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" @@ -1234,6 +1235,7 @@ func injectTableStats( } // Insert each statistic. +StatsLoop: for i := range jsonStats { s := &jsonStats[i] h, err := s.GetHistogram(¶ms.p.semaCtx, params.EvalContext()) @@ -1254,7 +1256,11 @@ func injectTableStats( for _, colName := range s.Columns { col, err := desc.FindColumnWithName(tree.Name(colName)) if err != nil { - return err + params.p.BufferClientNotice( + params.ctx, + pgnotice.Newf("column %q does not exist", colName), + ) + continue StatsLoop } if err := columnIDs.Append(tree.NewDInt(tree.DInt(col.GetID()))); err != nil { return err diff --git a/pkg/sql/logictest/testdata/logic_test/distsql_stats b/pkg/sql/logictest/testdata/logic_test/distsql_stats index ca78f57c67c9..6f06afd9b229 100644 --- a/pkg/sql/logictest/testdata/logic_test/distsql_stats +++ b/pkg/sql/logictest/testdata/logic_test/distsql_stats @@ -197,6 +197,40 @@ statistics_name column_names row_count distinct_count null_count NULL {b} 256 4 0 s1 {a} 256 4 0 +# Ignore stats with non-existent columns. +query T noticetrace +ALTER TABLE data INJECT STATISTICS '[ + { + "columns": ["z"], + "created_at": "2018-05-01 1:00:00.00000+00:00", + "row_count": 10, + "distinct_count": 2 + }, + { + "columns": ["a", "z"], + "created_at": "2018-05-01 1:00:00.00000+00:00", + "row_count": 10, + "distinct_count": 2 + }, + { + "columns": ["a"], + "created_at": "2018-05-01 1:00:00.00000+00:00", + "row_count": 10, + "distinct_count": 2 + } +]' +---- +NOTICE: column "z" does not exist +NOTICE: column "z" does not exist + +query TTIII colnames +SELECT statistics_name, column_names, row_count, distinct_count, null_count +FROM [SHOW STATISTICS FOR TABLE data] +ORDER BY statistics_name, column_names::STRING +---- +statistics_name column_names row_count distinct_count null_count +NULL {a} 10 2 0 + # Test AS OF SYSTEM TIME # We're reading from timestamps that precede the GC thresholds, disable strict @@ -216,7 +250,6 @@ FROM [SHOW STATISTICS FOR TABLE data] ORDER BY statistics_name, column_names::STRING ---- statistics_name column_names row_count distinct_count null_count -NULL {b} 256 4 0 s2 {a} 256 4 0 #