Merge cockroachdb#72818 cockroachdb#73151 cockroachdb#73302 cockroach…

…db#73572 cockroachdb#73613

72818: sql: use Latest[Non]PrimaryIndexDescriptorVersion constants in more places r=mgartner a=mgartner

These constants have been moved from tabledesc to descpb to avoid import
cycles.

Release note: None


73151: sql: require valid TenantID in start_replication_stream r=rafiss a=stevendanna

MakeTenantID panics if someone tries to use 0 as a tenant ID. Here, we
return an error immediately in this case rather than crashing.

Fixes cockroachdb#71311

Release note: None

73302: sql: add index recommendation to EXPLAIN output r=nehageorge a=nehageorge

**indexrec: disable inverted index recommendations** 

Previously, we would erroneously give index recommendations of non-
inverted indexes for indexes that should be inverted. In this PR,
we omit index candidates that required inverted indexes. Specifically,
this includes JSON columns, array columns, and spatial data columns.

Release note: None

**sql: add index recommendation to EXPLAIN** 

Previously, the EXPLAIN output only showed the query plan. Since users
often use EXPLAIN to see why a query is poorly performing, it would
be useful for them to also see index recommendations to make the query
faster. In this commit, index recommendations are added to the bottom of
the EXPLAIN output. See the indexrec package under `pkg/sql/opt/indexrec`
to understand how index recommendations are generated.

Release note (sql change): The output of the EXPLAIN SQL statement has changed.
Below the plan, we now output index recommendations for the SQL statement being
"EXPLAIN-ed", if there are any. These index recommendations are indexes the
user could add or indexes they could replace to make the given query faster.

Closes cockroachdb#72761.

**indexrec: refactor index recommendation output**

Previously, we manually formatted the string for the index recommendation's
SQL output. This commit takes advantage of the existing functionality for
formatting SQL statements, specifically `CREATE INDEX` and `DROP INDEX`. This
will make it easier to add more types of index recommendations in the future.

Release note: None

73572: bazel: perform staticcheck checks in `nogo` r=rail a=rickystewart

Add a new binary `generate-staticcheck`, part of `dev generate bazel`,
that creates a package in `build/bazelutil/staticcheckanalyzers`
capturing a single `staticcheck` check. Then we capture the list of
analyzers in `build/bazelutil/staticcheckanalyzers/def.bzl` and add all
those checks to `nogo`.

However, we can't directly use the `Analyzer`s provided by the upstream
library, because they report *all* violations regardless of
`lint:ignore` directives. In order to address this we add a helper
library at `pkg/testutils/lint/passes/staticcheck` that munges the
`Analyzer`s to additionally pull the `lint:ignore` directives and ignore
any reported diagnostics that correspond to a `lint:ignore`.

This follows the example set by https://github.com/sluongng/staticcheck-codegen,
but we don't directly use that code, since we need to inject our own
stuff and we want to make sure the content here is pinned to the right
version of `staticcheck`.

We run the `staticcheck` and `simple` [checks](https://staticcheck.io/docs/checks).
`stylecheck` is still broken: cockroachdb#73568

Closes cockroachdb#68496.

Release note: None

73613: bazel: add stylecheck analyzers to nogo r=rail a=rickystewart

We skip the checks `ST1000`, `ST1003`, `ST1016`, `ST1020`, `ST1021`,
and `ST1022` since they're skipped by the default `staticcheck` config,
and our codebase has many violations of them.

Closes cockroachdb#73568.

Release note: None

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Steven Danna <[email protected]>
Co-authored-by: Neha George <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>

.github/CODEOWNERS

@@ -161,6 +161,7 @@
 /pkg/cmd/generate-metadata-tables/ @cockroachdb/sql-experience
 /pkg/cmd/generate-spatial-ref-sys/ @cockroachdb/geospatial
 /pkg/cmd/generate-test-suites/ @cockroachdb/dev-inf
+/pkg/cmd/generate-staticcheck/ @cockroachdb/dev-inf
 /pkg/cmd/geoviz/             @cockroachdb/geospatial
 /pkg/cmd/github-post/        @cockroachdb/test-eng
 /pkg/cmd/github-pull-request-make/ @cockroachdb/dev-inf

BUILD.bazel

@@ -7,6 +7,7 @@ exports_files([
 
 load("@bazel_gazelle//:def.bzl", "gazelle")
 load("@io_bazel_rules_go//go:def.bzl", "nogo")
+load("//build/bazelutil/staticcheckanalyzers:def.bzl", "STATICCHECK_CHECKS")
 
 # The following directives inform gazelle how to auto-generate BUILD.bazel
 # files throughout the repo. By including them here, we can run gazelle using
@@ -250,6 +251,6 @@ nogo(
             "//pkg/testutils/lint/passes/returnerrcheck",
             "//pkg/testutils/lint/passes/timer",
             "//pkg/testutils/lint/passes/unconvert",
-        ],
+        ] + STATICCHECK_CHECKS,
     }),
 )

build/bazelutil/bazel-generate.sh

@@ -9,6 +9,7 @@ set -euo pipefail
 
 CONTENTS=$(bazel run //pkg/cmd/mirror)
 echo "$CONTENTS" > DEPS.bzl
+bazel run pkg/cmd/generate-staticcheck --run_under="cd $PWD && "
 bazel run //:gazelle
 CONTENTS=$(bazel run //pkg/cmd/generate-test-suites --run_under="cd $PWD && ")
 echo "$CONTENTS" > pkg/BUILD.bazel