Merge #76394 #76415 #76420 #76437 #76442 #76450

76394: sql: use IndexFetchSpec in TableReader r=RaduBerinde a=RaduBerinde

#### row: replace fetcher args with IndexFetchSpec

This commit replaces the row fetcher table args with IndexFetchSpec.

Release note: None

#### sql: use IndexFetchSpec in TableReader

This commit removes the table descriptor from TableReader and replaces
it with an IndexFetchSpec.

Eventually, we will use the same `IndexFetchSpec` to form the columnar
batch directly in KV.

Release note: None


76415: dev: address a few paper cuts r=irfansharif a=irfansharif

See individual commits, works through a few of the known paper cuts with dev/bazel (#75453).

76420: sql: initialize tenant span config with rangefeeds enabled r=irfansharif a=ajwerner

Not doing this causes problems when we construct new tenants as they won't be
able to establish a rangefeed until their first full reconciliation completes
and then propagates.

Even this is not great. If the preceding range did not have rangefeeds enabled,
it would take a closed timestamp interval for this enablement to propagate.
Perhaps this is evidence that we should always carve out a span at the end of
the keyspace and set it to have rangefeeds enabled.

I'll leave that fix and testing of this to somebody else. Hope this is helpful
enough on its own. cc `@irfansharif`  and `@arulajmani.` I believe this problem
has been blocking `@RaduBerinde.`

Fixes #76331

Release note: None

76437: clisqlshell: handle interactive query cancellation r=rafiss a=knz

Fixes #76433. 

As of this PR, there's a bug in lib/pq which forces the session to
terminate when any query gets cancelled. We find this unacceptable
and we plan to fix it later.

Release note (cli change): The interactive SQL shell (`cockroach sql`,
`cockroach demo`) now supports interrupting a currently running
query with Ctrl+C, without losing access to the shell.

76442: roachtest: try to stabilize ORM nightlies r=RichardJCai a=rafiss

fixes #76428
fixes #76426
fixes #68363
touches #76231
touches #76016
touches #76011

Ruby-pg and psycopg2 tests started passing because we now support pgwire
cancel.

Other tests (Django, Hibernate, and PGJDBC) seem flaky after the testing
settings change, so try to use slightly more conservative values.

Release note: None

76450: ci: add extra logging of `$TC_SERVER_URL` in pebble metamorphic nightly r=nicktrav a=rickystewart

Release note: None

Co-authored-by: Radu Berinde <[email protected]>
Co-authored-by: irfan sharif <[email protected]>
Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>

.bazelrc

@@ -1,5 +1,3 @@
-try-import %workspace%/.bazelrc.user
-
 build --define gotags=bazel,gss
 build --experimental_proto_descriptor_sets_include_source_info
 build --incompatible_strict_action_env --incompatible_enable_cc_toolchain_resolution
@@ -51,4 +49,7 @@ build:dev --action_env=PATH
 build:dev --host_action_env=PATH
 
 build:nonogo --define cockroach_nonogo=y
+
+try-import %workspace%/.bazelrc.user
+
 # vi: ft=sh

build/teamcity/cockroach/nightlies/pebble_nightly_metamorphic.sh

@@ -23,18 +23,5 @@ echo "$NEW_DEPS_BZL_CONTENT" > DEPS.bzl
 PEBBLE_SUM=$(grep 'version =' DEPS.bzl | cut -d'"' -f2)
 echo "Pebble module sum: $PEBBLE_SUM"
 
-env=(
-  "GITHUB_REPO=$GITHUB_REPO"
-  "GITHUB_API_TOKEN=$GITHUB_API_TOKEN"
-  "BUILD_VCS_NUMBER=$PEBBLE_SUM"
-  "TC_BUILD_ID=$TC_BUILD_ID"
-  "TC_SERVER_URL=$TC_SERVER_URL"
-  "TC_BUILD_BRANCH=$TC_BUILD_BRANCH"
-  "PKG=internal/metamorphic"
-  "STRESSFLAGS=-maxtime 3h -maxfails 1 -stderr -p 1"
-  "TZ=America/New_York"
-)
-
 BAZEL_SUPPORT_EXTRA_DOCKER_ARGS="-e BUILD_VCS_NUMBER=$PEBBLE_SUM -e GITHUB_API_TOKEN -e GITHUB_REPO -e TC_BUILD_BRANCH -e TC_BUILD_ID -e TC_SERVER_URL" \
                                run_bazel build/teamcity/cockroach/nightlies/pebble_nightly_metamorphic_impl.sh
-

build/teamcity/cockroach/nightlies/pebble_nightly_metamorphic_impl.sh

@@ -8,6 +8,8 @@ ARTIFACTS_DIR=/artifacts/meta
 mkdir -p $ARTIFACTS_DIR
 GO_TEST_JSON_OUTPUT_FILE=/artifacts/test.json.txt
 
+echo "TC_SERVER_URL is $TC_SERVER_URL"
+
 bazel build //pkg/cmd/bazci //pkg/cmd/github-post //pkg/cmd/testfilter --config=ci
 
 BAZEL_BIN=$(bazel info bazel-bin --config ci)

dev

@@ -3,7 +3,7 @@
 set -euo pipefail
 
 # Bump this counter to force rebuilding `dev` on all machines.
-DEV_VERSION=15
+DEV_VERSION=16
 
 THIS_DIR=$(cd "$(dirname "$0")" && pwd)
 BINARY_DIR=$THIS_DIR/bin/dev-versions

go.mod

@@ -163,6 +163,8 @@ require (
 	vitess.io/vitess v0.0.0-00010101000000-000000000000
 )
 
+require github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
+
 require (
 	cloud.google.com/go v0.100.2 // indirect
 	cloud.google.com/go/compute v1.1.0 // indirect

go.sum

@@ -1067,6 +1067,7 @@ github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1/go.mod h1:kpwsk12EmLe
 github.com/google/pprof v0.0.0-20210827144239-02619b876842 h1:JCrt5MIE1fHQtdy1825HwJ45oVQaqHE6lgssRhjcg/o=
 github.com/google/pprof v0.0.0-20210827144239-02619b876842/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
 github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
+github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4=
 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ=
 github.com/google/skylark v0.0.0-20181101142754-a5f7082aabed h1:rZdD1GeRTHD1aG+VIvhQEYXurx6Wfg4QIT5YVl2tSC8=
 github.com/google/skylark v0.0.0-20181101142754-a5f7082aabed/go.mod h1:CKSX6SxHW1vp20ZNaeGe3TFFBIwCG6vaYrpAiOzX+NA=

pkg/ccl/changefeedccl/rowfetcher_cache.go

@@ -164,21 +164,22 @@ func (c *rowFetcherCache) RowFetcherForTableDesc(
 	rf := &f.fetcher
 
 	// TODO(dan): Allow for decoding a subset of the columns.
-	rfArgs := row.FetcherTableArgs{
-		Desc:    tableDesc,
-		Index:   tableDesc.GetPrimaryIndex(),
-		Columns: tableDesc.PublicColumns(),
+	var spec descpb.IndexFetchSpec
+	if err := rowenc.InitIndexFetchSpec(
+		&spec, c.codec, tableDesc, tableDesc.GetPrimaryIndex(), tableDesc.PublicColumnIDs(),
+	); err != nil {
+		return nil, err
 	}
+
 	if err := rf.Init(
 		context.TODO(),
-		c.codec,
 		false, /* reverse */
 		descpb.ScanLockingStrength_FOR_NONE,
 		descpb.ScanLockingWaitPolicy_BLOCK,
 		0, /* lockTimeout */
 		&c.a,
 		nil, /* memMonitor */
-		rfArgs,
+		&spec,
 	); err != nil {
 		return nil, err
 	}

pkg/ccl/cliccl/BUILD.bazel

@@ -33,12 +33,12 @@ go_library(
         "//pkg/security",
         "//pkg/server",
         "//pkg/settings/cluster",
-        "//pkg/sql/catalog",
         "//pkg/sql/catalog/catconstants",
         "//pkg/sql/catalog/colinfo",
         "//pkg/sql/catalog/descpb",
         "//pkg/sql/catalog/tabledesc",
         "//pkg/sql/row",
+        "//pkg/sql/rowenc",
         "//pkg/sql/sem/tree",
         "//pkg/storage",
         "//pkg/storage/enginepb",

pkg/ccl/cliccl/debug_backup.go

@@ -38,12 +38,12 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/server"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
-	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catconstants"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/row"
+	"github.com/cockroachdb/cockroach/pkg/sql/rowenc"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/storage"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
@@ -563,36 +563,26 @@ func makeIters(
 func makeRowFetcher(
 	ctx context.Context, entry backupccl.BackupTableEntry, codec keys.SQLCodec,
 ) (row.Fetcher, error) {
-	colDescs := make([]catalog.Column, len(entry.Desc.PublicColumns()))
-	for i, col := range entry.Desc.PublicColumns() {
-		colDescs[i] = col
-	}
-
+	colIDs := entry.Desc.PublicColumnIDs()
 	if debugBackupArgs.withRevisions {
-		newCol, err := entry.Desc.FindColumnWithName(colinfo.MVCCTimestampColumnName)
-		if err != nil {
-			return row.Fetcher{}, errors.Wrapf(err, "get mvcc timestamp column")
-		}
-		colDescs = append(colDescs, newCol)
+		colIDs = append(colIDs, colinfo.MVCCTimestampColumnID)
 	}
 
-	table := row.FetcherTableArgs{
-		Desc:    entry.Desc,
-		Index:   entry.Desc.GetPrimaryIndex(),
-		Columns: colDescs,
+	var spec descpb.IndexFetchSpec
+	if err := rowenc.InitIndexFetchSpec(&spec, codec, entry.Desc, entry.Desc.GetPrimaryIndex(), colIDs); err != nil {
+		return row.Fetcher{}, err
 	}
 
 	var rf row.Fetcher
 	if err := rf.Init(
 		ctx,
-		codec,
 		false, /*reverse*/
 		descpb.ScanLockingStrength_FOR_NONE,
 		descpb.ScanLockingWaitPolicy_BLOCK,
 		0, /* lockTimeout */
 		&tree.DatumAlloc{},
 		nil, /*mon.BytesMonitor*/
-		table,
+		&spec,
 	); err != nil {
 		return rf, err
 	}

pkg/ccl/spanconfigccl/spanconfigcomparedccl/testdata/multitenant

@@ -19,16 +19,14 @@ configs version=current offset=43
 ----
 ...
 /Table/50                                  database system (host)
-/Tenant/10                                 range default
-/Tenant/11                                 range default
+/Tenant/10                                 database system (tenant)
+/Tenant/11                                 database system (tenant)
 
 diff offset=50
 ----
 --- gossiped system config span (legacy)
 +++ span config infrastructure (current)
 ...
-+/Tenant/10                                 range default
-+/Tenant/11                                 range default
 
 reconcile tenant=11
 ----
@@ -40,7 +38,7 @@ configs version=current offset=43 limit=5
 ----
 ...
 /Table/50                                  database system (host)
-/Tenant/10                                 range default
+/Tenant/10                                 database system (tenant)
 /Tenant/11                                 database system (tenant)
 /Tenant/11/Table/4                         database system (tenant)
 /Tenant/11/Table/5                         database system (tenant)
@@ -58,8 +56,7 @@ diff offset=48 limit=10
 --- gossiped system config span (legacy)
 +++ span config infrastructure (current)
 ...
-+/Table/50                                  database system (host)
-+/Tenant/10                                 range default
+ /Tenant/10                                 database system (tenant)
  /Tenant/11                                 database system (tenant)
 +/Tenant/11/Table/4                         database system (tenant)
 +/Tenant/11/Table/5                         database system (tenant)
@@ -68,6 +65,7 @@ diff offset=48 limit=10
 +/Tenant/11/Table/11                        database system (tenant)
 +/Tenant/11/Table/12                        database system (tenant)
 +/Tenant/11/Table/13                        database system (tenant)
++/Tenant/11/Table/14                        database system (tenant)
  ...
 
 # Sanity check that new tenant tables show up correctly.
@@ -84,8 +82,7 @@ diff offset=48
 --- gossiped system config span (legacy)
 +++ span config infrastructure (current)
 ...
-+/Table/50                                  database system (host)
-+/Tenant/10                                 range default
+ /Tenant/10                                 database system (tenant)
  /Tenant/11                                 database system (tenant)
 +/Tenant/11/Table/4                         database system (tenant)
 +/Tenant/11/Table/5                         database system (tenant)

pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/multitenant/basic

@@ -23,8 +23,8 @@ state offset=47
 ----
 ...
 /Table/5{0-1}                              database system (host)
-/Tenant/10{-"\x00"}                        range default
-/Tenant/11{-"\x00"}                        range default
+/Tenant/10{-"\x00"}                        database system (tenant)
+/Tenant/11{-"\x00"}                        database system (tenant)
 
 # Start the reconciliation loop for the secondary tenant.
 reconcile tenant=10
@@ -106,7 +106,7 @@ state offset=47
 /Tenant/10/Table/4{3-4}                    database system (tenant)
 /Tenant/10/Table/4{4-5}                    database system (tenant)
 /Tenant/10/Table/4{6-7}                    database system (tenant)
-/Tenant/11{-"\x00"}                        range default
+/Tenant/11{-"\x00"}                        database system (tenant)
 
 exec-sql tenant=10
 CREATE DATABASE db;
@@ -125,4 +125,4 @@ state offset=81
 /Tenant/10/Table/4{6-7}                    database system (tenant)
 /Tenant/10/Table/10{6-7}                   range default
 /Tenant/10/Table/10{7-8}                   range default
-/Tenant/11{-"\x00"}                        range default
+/Tenant/11{-"\x00"}                        database system (tenant)

pkg/cli/clisqlshell/BUILD.bazel

@@ -26,6 +26,7 @@ go_library(
         "//pkg/sql/scanner",
         "//pkg/sql/sqlfsm",
         "//pkg/util/envutil",
+        "//pkg/util/syncutil",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_knz_go_libedit//:go-libedit",
     ],

pkg/cli/clisqlshell/context.go

@@ -15,6 +15,7 @@ import (
 	"time"
 
 	democlusterapi "github.com/cockroachdb/cockroach/pkg/cli/democluster/api"
+	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 )
 
 // Context represents the external configuration of the interactive
@@ -71,4 +72,11 @@ type internalContext struct {
 
 	// current database name, if known. This is maintained on a best-effort basis.
 	dbName string
+
+	// state about the current query.
+	mu struct {
+		syncutil.Mutex
+		cancelFn func()
+		doneCh   chan struct{}
+	}
 }