Default LogicalColumnID to ColumnID when not set

cockroachdb · Apr 7, 2020 · 33d9dbc · 33d9dbc
1 parent fed431e
commit 33d9dbc
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 9 deletions.
diff --git a/pkg/sql/information_schema.go b/pkg/sql/information_schema.go
@@ -391,7 +391,7 @@ https://www.postgresql.org/docs/9.5/infoschema-columns.html`,
 					scNameStr,                    // table_schema
 					tree.NewDString(table.Name),  // table_name
 					tree.NewDString(column.Name), // column_name
-					tree.NewDInt(tree.DInt(column.LogicalColumnID)),      // ordinal_position
+					tree.NewDInt(tree.DInt(column.GetLogicalColumnID())), // ordinal_position
 					dStringPtrOrNull(column.DefaultExpr),                 // column_default
 					yesOrNoDatum(column.Nullable),                        // is_nullable
 					tree.NewDString(column.Type.InformationSchemaName()), // data_type

diff --git a/pkg/sql/pg_catalog.go b/pkg/sql/pg_catalog.go
@@ -406,9 +406,9 @@ CREATE TABLE pg_catalog.pg_attrdef (
 						defSrc = tree.NewDString(ctx.String())
 					}
 					return addRow(
-						h.ColumnOid(table.ID, column.ID),                // oid
-						defaultOid(table.ID),                            // adrelid
-						tree.NewDInt(tree.DInt(column.LogicalColumnID)), // adnum
+						h.ColumnOid(table.ID, column.ID),                     // oid
+						defaultOid(table.ID),                                 // adrelid
+						tree.NewDInt(tree.DInt(column.GetLogicalColumnID())), // adnum
 						defSrc, // adbin
 						defSrc, // adsrc
 					)
@@ -495,7 +495,7 @@ CREATE TABLE pg_catalog.pg_attribute (
 			// Columns for table.
 			if err := forEachColumnInTable(table, func(column *sqlbase.ColumnDescriptor) error {
 				tableID := defaultOid(table.ID)
-				return addColumn(column, tableID, column.LogicalColumnID)
+				return addColumn(column, tableID, column.GetLogicalColumnID())
 			}); err != nil {
 				return err
 			}
@@ -505,7 +505,7 @@ CREATE TABLE pg_catalog.pg_attribute (
 				return forEachColumnInIndex(table, index,
 					func(column *sqlbase.ColumnDescriptor) error {
 						idxID := h.IndexOid(table.ID, index.ID)
-						return addColumn(column, idxID, column.LogicalColumnID)
+						return addColumn(column, idxID, column.GetLogicalColumnID())
 					},
 				)
 			})

diff --git a/pkg/sql/sqlbase/structured.go b/pkg/sql/sqlbase/structured.go
@@ -4096,3 +4096,13 @@ func GenerateUniqueConstraintName(prefix string, nameExistsFunc func(name string
 	}
 	return name
 }
+
+// GetLogicalColumnID returns the LogicalColumnID if it has one, otherwise
+// returns the ID. The regular ColumnID is used for backwards compatibility.
+func (desc *ColumnDescriptor) GetLogicalColumnID() ColumnID {
+	if desc.LogicalColumnID != 0 {
+		return desc.LogicalColumnID
+	}
+
+	return desc.ID
+}
diff --git a/pkg/sql/sqlbase/structured_test.go b/pkg/sql/sqlbase/structured_test.go
@@ -80,9 +80,9 @@ func TestAllocateIDs(t *testing.T) {
 		Version:  1,
 		Name:     "foo",
 		Columns: []ColumnDescriptor{
-			{ID: 1, Name: "a"},
-			{ID: 2, Name: "b"},
-			{ID: 3, Name: "c"},
+			{ID: 1, Name: "a", LogicalColumnID: 1},
+			{ID: 2, Name: "b", LogicalColumnID: 2},
+			{ID: 3, Name: "c", LogicalColumnID: 3},
 		},
 		Families: []ColumnFamilyDescriptor{
 			{
@@ -1369,3 +1369,22 @@ func TestSQLString(t *testing.T) {
 		t.Errorf("Expected '%s', but got '%s'", expected, got)
 	}
 }
+
+func TestLogicalColumnID(t *testing.T) {
+	tests := []struct {
+		desc     ColumnDescriptor
+		expected ColumnID
+	}{
+		{ColumnDescriptor{ID: 1, LogicalColumnID: 1}, 1},
+		{ColumnDescriptor{ID: 2}, 2},
+	}
+
+	for i := range tests {
+		actual := tests[i].desc.GetLogicalColumnID()
+		expected := tests[i].expected
+
+		if expected != actual {
+			t.Fatalf("Expected LogicalColumnID to be %d, got %d.", expected, actual)
+		}
+	}
+}