Skip to content

Commit

Permalink
Merge #78997
Browse files Browse the repository at this point in the history
78997: sql: ignore non-existent columns when injecting stats r=mgartner a=mgartner

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.

Co-authored-by: Marcus Gartner <[email protected]>
  • Loading branch information
craig[bot] and mgartner committed Mar 30, 2022
2 parents a2d43c8 + b3bef41 commit a0632ce
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 2 deletions.
8 changes: 7 additions & 1 deletion pkg/sql/alter_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -1234,6 +1235,7 @@ func injectTableStats(
}

// Insert each statistic.
StatsLoop:
for i := range jsonStats {
s := &jsonStats[i]
h, err := s.GetHistogram(&params.p.semaCtx, params.EvalContext())
Expand All @@ -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
Expand Down
35 changes: 34 additions & 1 deletion pkg/sql/logictest/testdata/logic_test/distsql_stats
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

#
Expand Down

0 comments on commit a0632ce

Please sign in to comment.