Merge #75426 #75427 #75440

75426: catalog: minor improvements to system columns r=RaduBerinde a=RaduBerinde

This change cleans up the definition of system column IDs and uses the
column ID to determine the SystemColumnKind more efficiently.

Release note: None

75427: catalog: add Index.InvertedColumnKeyType r=RaduBerinde a=RaduBerinde

In an inverted index, the encoded key is treated as a Bytes datum.
This is currently hardcoded in the opt catalog.

This commit moves this into the index descriptor.

Release note: None

75440: roachtest: new passing tests for ruby-pg and psycopg2 r=ZhouXing19 a=rafiss

fixes #75309
fixes #75310

The recent SCRAM work made 3 tests pass.

Release note: None

Co-authored-by: Radu Berinde <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>

pkg/cmd/roachtest/tests/psycopg_blocklist.go

@@ -28,7 +28,11 @@ var psycopgBlocklists = blocklistsForVersion{
 // Please keep these lists alphabetized for easy diffing.
 // After a failed run, an updated version of this blocklist should be available
 // in the test log.
-var psycopgBlockList22_1 = psycopgBlockList21_2
+var psycopgBlockList22_1 = blocklist{
+	"tests.test_async_keyword.CancelTests.test_async_cancel": "41335",
+	// The following item can be removed once there is a new psycopg2 release.
+	"tests.test_module.ExceptionsTestCase.test_9_6_diagnostics": "58035",
+}
 
 var psycopgBlockList21_2 = blocklist{
 	"tests.test_async_keyword.CancelTests.test_async_cancel": "41335",

pkg/cmd/roachtest/tests/ruby_pg_blocklist.go

@@ -95,7 +95,6 @@ var rubyPGBlockList22_1 = blocklist{
 	"PG::Connection multinationalization support rubyforge #22925: m17n support uses the previous string encoding for escaped string":                                                                                           "unknown",
 	"PG::Connection multinationalization support rubyforge #22925: m17n support uses the previous string encoding for quote_ident":                                                                                              "unknown",
 	"PG::Connection not read past the end of a large object":                                                                                                                                                                    "unknown",
-	"PG::Connection password encryption method can encrypt without algorithm":                                                                                                                                                   "unknown",
 	"PG::Connection returns notifications which are already in the queue before wait_for_notify is called without waiting for the socket to become readable":                                                                    "unknown",
 	"PG::Connection returns the block result from Connection#transaction":                                                                                                                                                       "unknown",
 	"PG::Connection sends nil as the payload if the notification wasn't given one":                                                                                                                                              "unknown",
@@ -173,7 +172,6 @@ var rubyPGBlockList22_1 = blocklist{
 	"running with sync_* methods PG::Connection multinationalization support rubyforge #22925: m17n support uses the previous string encoding for escaped string":                                                               "unknown",
 	"running with sync_* methods PG::Connection multinationalization support rubyforge #22925: m17n support uses the previous string encoding for quote_ident":                                                                  "unknown",
 	"running with sync_* methods PG::Connection not read past the end of a large object":                                                                                                                                        "unknown",
-	"running with sync_* methods PG::Connection password encryption method can encrypt without algorithm":                                                                                                                       "unknown",
 	"running with sync_* methods PG::Connection returns notifications which are already in the queue before wait_for_notify is called without waiting for the socket to become readable":                                        "unknown",
 	"running with sync_* methods PG::Connection returns the block result from Connection#transaction":                                                                                                                           "unknown",
 	"running with sync_* methods PG::Connection sends nil as the payload if the notification wasn't given one":                                                                                                                  "unknown",

pkg/sql/catalog/catalog.go

@@ -107,11 +107,11 @@ func (p ResolvedObjectPrefix) NamePrefix() tree.ObjectNamePrefix {
 }
 
 // NumSystemColumns defines the number of supported system columns and must be
-// equal to len(colinfo.AllSystemColumnDescs) (enforced in colinfo package to
-// avoid an import cycle).
+// equal to colinfo.numSystemColumns (enforced in colinfo package to avoid an
+// import cycle).
 const NumSystemColumns = 2
 
 // SmallestSystemColumnColumnID is a descpb.ColumnID with the smallest value
 // among all system columns (enforced in colinfo package to avoid an import
 // cycle).
-const SmallestSystemColumnColumnID = math.MaxUint32 - 1
+const SmallestSystemColumnColumnID = math.MaxUint32 - NumSystemColumns + 1

pkg/sql/catalog/colinfo/system_columns.go

@@ -33,30 +33,39 @@ import (
 // * tableoid: A Postgres system column that contains the ID of the table
 //   that a particular row came from.
 
-// AllSystemColumnDescs contains all registered system columns.
-var AllSystemColumnDescs = []descpb.ColumnDescriptor{
-	MVCCTimestampColumnDesc,
-	TableOIDColumnDesc,
-}
+const (
+	// MVCCTimestampColumnID is the ColumnID of the MVCC timestamp column.
+	MVCCTimestampColumnID descpb.ColumnID = math.MaxUint32 - iota
 
-// MVCCTimestampColumnID is the ColumnID of the MVCC timesatmp column. Future
-// system columns will have ID's that decrement from this value.
-const MVCCTimestampColumnID = math.MaxUint32
+	// TableOIDColumnID is the ID of the tableoid system column.
+	TableOIDColumnID
 
-// TableOIDColumnID is the ID of the tableoid system column.
-const TableOIDColumnID = MVCCTimestampColumnID - 1
+	numSystemColumns = iota
+)
 
 func init() {
-	if len(AllSystemColumnDescs) != catalog.NumSystemColumns {
+	if len(AllSystemColumnDescs) != numSystemColumns {
+		panic("need to update system column IDs or descriptors")
+	}
+	if catalog.NumSystemColumns != numSystemColumns {
 		panic("need to update catalog.NumSystemColumns")
 	}
+	if catalog.SmallestSystemColumnColumnID != math.MaxUint32-numSystemColumns+1 {
+		panic("need to update catalog.SmallestSystemColumnColumnID")
+	}
 	for _, desc := range AllSystemColumnDescs {
-		if desc.ID < catalog.SmallestSystemColumnColumnID {
-			panic("need to update catalog.SmallestSystemColumnColumnID")
+		if desc.SystemColumnKind != GetSystemColumnKindFromColumnID(desc.ID) {
+			panic("system column ID ordering must match SystemColumnKind value ordering")
 		}
 	}
 }
 
+// AllSystemColumnDescs contains all registered system columns.
+var AllSystemColumnDescs = []descpb.ColumnDescriptor{
+	MVCCTimestampColumnDesc,
+	TableOIDColumnDesc,
+}
+
 // MVCCTimestampColumnDesc is a column descriptor for the MVCC system column.
 var MVCCTimestampColumnDesc = descpb.ColumnDescriptor{
 	Name:             MVCCTimestampColumnName,
@@ -88,18 +97,17 @@ const TableOIDColumnName = "tableoid"
 
 // IsColIDSystemColumn returns whether a column ID refers to a system column.
 func IsColIDSystemColumn(colID descpb.ColumnID) bool {
-	return GetSystemColumnKindFromColumnID(colID) != catpb.SystemColumnKind_NONE
+	return colID > math.MaxUint32-numSystemColumns
 }
 
 // GetSystemColumnKindFromColumnID returns the kind of system column that colID
 // refers to.
 func GetSystemColumnKindFromColumnID(colID descpb.ColumnID) catpb.SystemColumnKind {
-	for i := range AllSystemColumnDescs {
-		if AllSystemColumnDescs[i].ID == colID {
-			return AllSystemColumnDescs[i].SystemColumnKind
-		}
+	i := math.MaxUint32 - uint32(colID)
+	if i >= numSystemColumns {
+		return catpb.SystemColumnKind_NONE
 	}
-	return catpb.SystemColumnKind_NONE
+	return catpb.SystemColumnKind_NONE + 1 + catpb.SystemColumnKind(i)
 }
 
 // IsSystemColumnName returns whether or not a name is a reserved system

pkg/sql/catalog/descpb/index.go

@@ -14,6 +14,7 @@ import (
 	"fmt"
 
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
+	types "github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/errors"
 )
 
@@ -112,3 +113,14 @@ func (desc *IndexDescriptor) InvertedColumnName() string {
 	}
 	return desc.KeyColumnNames[len(desc.KeyColumnNames)-1]
 }
+
+// InvertedColumnKeyType returns the type of the data element that is encoded
+// as the inverted index key. This is currently always Bytes.
+//
+// Panics if the index is not inverted.
+func (desc *IndexDescriptor) InvertedColumnKeyType() *types.T {
+	if desc.Type != IndexDescriptor_INVERTED {
+		panic(errors.AssertionFailedf("index is not inverted"))
+	}
+	return types.Bytes
+}

pkg/sql/catalog/table_elements.go

@@ -152,9 +152,24 @@ type Index interface {
 	CollectSecondaryStoredColumnIDs() TableColSet
 	CollectCompositeColumnIDs() TableColSet
 
+	// InvertedColumnID returns the ColumnID of the inverted column of the
+	// inverted index.
+	//
+	// Panics if the index is not inverted.
 	InvertedColumnID() descpb.ColumnID
+
+	// InvertedColumnName returns the name of the inverted column of the inverted
+	// index.
+	//
+	// Panics if the index is not inverted.
 	InvertedColumnName() string
 
+	// InvertedColumnKeyType returns the type of the data element that is encoded
+	// as the inverted index key. This is currently always Bytes.
+	//
+	// Panics if the index is not inverted.
+	InvertedColumnKeyType() *types.T
+
 	NumPrimaryStoredColumns() int
 	NumSecondaryStoredColumns() int
 	GetStoredColumnID(storedColumnOrdinal int) descpb.ColumnID

pkg/sql/catalog/tabledesc/index.go

@@ -18,6 +18,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
+	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/util/iterutil"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
 )
@@ -160,6 +161,14 @@ func (w index) InvertedColumnName() string {
 	return w.desc.InvertedColumnName()
 }
 
+// InvertedColumnKeyType returns the type of the data element that is encoded
+// as the inverted index key. This is currently always Bytes.
+//
+// Panics if the index is not inverted.
+func (w index) InvertedColumnKeyType() *types.T {
+	return w.desc.InvertedColumnKeyType()
+}
+
 // CollectKeyColumnIDs creates a new set containing the column IDs in the key
 // of this index.
 func (w index) CollectKeyColumnIDs() catalog.TableColSet {

pkg/sql/opt_catalog.go

@@ -806,22 +806,25 @@ func newOptTable(
 			}
 		}
 		if idx.GetType() == descpb.IndexDescriptor_INVERTED {
-			// The last column of an inverted index is special: in the
+			// The inverted column of an inverted index is special: in the
 			// descriptors, it looks as if the table column is part of the
 			// index; in fact the key contains values *derived* from that
 			// column. In the catalog, we refer to this key as a separate,
 			// inverted column.
-			invertedSourceColOrdinal, _ := ot.lookupColumnOrdinal(idx.GetKeyColumnID(idx.NumKeyColumns() - 1))
+			invertedColumnID := idx.InvertedColumnID()
+			invertedColumnName := idx.InvertedColumnName()
+			invertedColumnType := idx.InvertedColumnKeyType()
+
+			invertedSourceColOrdinal, _ := ot.lookupColumnOrdinal(invertedColumnID)
 
 			// Add a inverted column that refers to the inverted index key.
 			invertedCol, invertedColOrd := newColumn()
 
 			// All inverted columns have type bytes.
-			typ := types.Bytes
 			invertedCol.InitInverted(
 				invertedColOrd,
-				tree.Name(string(ot.Column(invertedSourceColOrdinal).ColName())+"_inverted_key"),
-				typ,
+				tree.Name(invertedColumnName+"_inverted_key"),
+				invertedColumnType,
 				false, /* nullable */
 				invertedSourceColOrdinal,
 			)