Skip to content

Commit

Permalink
Merge #57803
Browse files Browse the repository at this point in the history 
57803: sql: initial support for virtual columns r=RaduBerinde a=RaduBerinde

This change adds very basic support for virtual columns to the
non-test descriptors and catalog. This is gated behind an experimental
session setting.

Informs #57608.

Release note: None

Co-authored-by: Radu Berinde <[email protected]>
  • Loading branch information
@RaduBerinde
craig[bot] and RaduBerinde committed Dec 12, 2020
2 parents 3954691 + 09fd267 commit 960b4cf
Show file tree
Hide file tree
Showing 21 changed files with 665 additions and 387 deletions.
2 changes: 1 addition & 1 deletion docs/generated/settings/settings.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,6 @@
<tr><td><code>trace.debug.enable</code></td><td>boolean</td><td><code>false</code></td><td>if set, traces for recent requests can be seen in the /debug page</td></tr>
<tr><td><code>trace.lightstep.token</code></td><td>string</td><td><code></code></td><td>if set, traces go to Lightstep using this token</td></tr>
<tr><td><code>trace.zipkin.collector</code></td><td>string</td><td><code></code></td><td>if set, traces go to the given Zipkin instance (example: '127.0.0.1:9411'); ignored if trace.lightstep.token is set</td></tr>
<tr><td><code>version</code></td><td>version</td><td><code>20.2-6</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
<tr><td><code>version</code></td><td>version</td><td><code>20.2-8</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
</tbody>
</table>
6 changes: 6 additions & 0 deletions pkg/clusterversion/cockroach_versions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,8 @@ const (
// UniqueWithoutIndexConstraints is when adding UNIQUE WITHOUT INDEX
// constraints is supported.
UniqueWithoutIndexConstraints
// VirtualComputedColumns is when virtual computed columns are supported.
VirtualComputedColumns

// Step (1): Add new versions here.
)
Expand Down Expand Up @@ -328,6 +330,10 @@ var versionsSingleton = keyedVersions([]keyedVersion{
Key: UniqueWithoutIndexConstraints,
Version: roachpb.Version{Major: 20, Minor: 2, Internal: 6},
},
{
Key: VirtualComputedColumns,
Version: roachpb.Version{Major: 20, Minor: 2, Internal: 8},
},

// Step (2): Add new versions here.
})
Expand Down
5 changes: 3 additions & 2 deletions pkg/clusterversion/key_string.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

715 changes: 376 additions & 339 deletions pkg/sql/catalog/descpb/structured.pb.go
View file

Large diffs are not rendered by default.

8 changes: 7 additions & 1 deletion pkg/sql/catalog/descpb/structured.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ message ColumnDescriptor {
option (gogoproto.equal) = true;
optional string name = 1 [(gogoproto.nullable) = false];
optional uint32 id = 2 [(gogoproto.nullable) = false,
(gogoproto.customname) = "ID", (gogoproto.casttype) = "ColumnID"];
(gogoproto.customname) = "ID",
(gogoproto.casttype) = "ColumnID"];
optional sql.sem.types.T type = 3;
optional bool nullable = 4 [(gogoproto.nullable) = false];
reserved 8;
Expand All @@ -142,6 +143,11 @@ message ColumnDescriptor {
// have been serialized in a internal format. Instead, use one of the
// schemaexpr.FormatExpr* functions.
optional string compute_expr = 11;

// A computed column can be stored or virtual.
// Virtual can only be true if there is a compute expression.
optional bool virtual = 16 [(gogoproto.nullable) = false];

// PGAttributeNum must be accessed through the accessor, since it is set
// lazily, it is incorrect to access it directly.
// PGAttributeNum represents a column's number in catalog tables.
Expand Down
6 changes: 5 additions & 1 deletion pkg/sql/catalog/schemaexpr/column.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,11 @@ func FormatColumnForDisplay(
return "", err
}
f.WriteString(compExpr)
f.WriteString(") STORED")
if desc.Virtual {
f.WriteString(") VIRTUAL")
} else {
f.WriteString(") STORED")
}
}
return f.CloseAndGetString(), nil
}
Expand Down
48 changes: 33 additions & 15 deletions pkg/sql/catalog/tabledesc/structured.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1316,6 +1316,10 @@ func (desc *Mutable) allocateColumnFamilyIDs(columnNames map[string]descpb.Colum
}

ensureColumnInFamily := func(col *descpb.ColumnDescriptor) {
if col.Virtual {
// Virtual columns don't need to be part of families.
return
}
if columnsInFamilies.Contains(col.ID) {
return
}
Expand Down Expand Up @@ -1944,7 +1948,7 @@ func (desc *Immutable) ValidateTable(ctx context.Context) error {
}

columnNames := make(map[string]descpb.ColumnID, len(desc.Columns))
columnIDs := make(map[descpb.ColumnID]string, len(desc.Columns))
columnIDs := make(map[descpb.ColumnID]*descpb.ColumnDescriptor, len(desc.Columns))
if err := desc.validateColumns(columnNames, columnIDs); err != nil {
return err
}
Expand All @@ -1959,7 +1963,7 @@ func (desc *Immutable) ValidateTable(ctx context.Context) error {
"mutation in state %s, direction %s, col %q, id %v",
errors.Safe(m.State), errors.Safe(m.Direction), col.Name, errors.Safe(col.ID))
}
columnIDs[col.ID] = col.Name
columnIDs[col.ID] = col
case *descpb.DescriptorMutation_Index:
if unSetEnums {
idx := desc.Index
Expand Down Expand Up @@ -2080,9 +2084,12 @@ func (desc *Immutable) ValidateTable(ctx context.Context) error {
}

func (desc *Immutable) validateColumns(
columnNames map[string]descpb.ColumnID, columnIDs map[descpb.ColumnID]string,
columnNames map[string]descpb.ColumnID, columnIDs map[descpb.ColumnID]*descpb.ColumnDescriptor,
) error {
for _, column := range desc.AllNonDropColumns() {
colDescs := desc.AllNonDropColumns()
for colIdx := range colDescs {
column := &colDescs[colIdx]

if err := catalog.ValidateName(column.Name, "column"); err != nil {
return err
}
Expand All @@ -2108,9 +2115,9 @@ func (desc *Immutable) validateColumns(

if other, ok := columnIDs[column.ID]; ok {
return fmt.Errorf("column %q duplicate ID of column %q: %d",
column.Name, other, column.ID)
column.Name, other.Name, column.ID)
}
columnIDs[column.ID] = column.Name
columnIDs[column.ID] = column

if column.ID >= desc.NextColumnID {
return errors.AssertionFailedf("column %q invalid ID (%d) >= next column ID (%d)",
Expand All @@ -2131,12 +2138,16 @@ func (desc *Immutable) validateColumns(
return fmt.Errorf("computed column %q refers to unknown columns in expression: %s",
column.Name, *column.ComputeExpr)
}
} else if column.Virtual {
return fmt.Errorf("virtual column %q is not computed", column.Name)
}
}
return nil
}

func (desc *Immutable) validateColumnFamilies(columnIDs map[descpb.ColumnID]string) error {
func (desc *Immutable) validateColumnFamilies(
columnIDs map[descpb.ColumnID]*descpb.ColumnDescriptor,
) error {
if len(desc.Families) < 1 {
return fmt.Errorf("at least 1 column family must be specified")
}
Expand Down Expand Up @@ -2184,13 +2195,16 @@ func (desc *Immutable) validateColumnFamilies(columnIDs map[descpb.ColumnID]stri
}

for i, colID := range family.ColumnIDs {
name, ok := columnIDs[colID]
col, ok := columnIDs[colID]
if !ok {
return fmt.Errorf("family %q contains unknown column \"%d\"", family.Name, colID)
}
if name != family.ColumnNames[i] {
if col.Name != family.ColumnNames[i] {
return fmt.Errorf("family %q column %d should have name %q, but found name %q",
family.Name, colID, name, family.ColumnNames[i])
family.Name, colID, col.Name, family.ColumnNames[i])
}
if col.Virtual {
return fmt.Errorf("virtual computed column %q cannot be part of a family", col.Name)
}
}

Expand All @@ -2201,9 +2215,11 @@ func (desc *Immutable) validateColumnFamilies(columnIDs map[descpb.ColumnID]stri
colIDToFamilyID[colID] = family.ID
}
}
for colID := range columnIDs {
if _, ok := colIDToFamilyID[colID]; !ok {
return fmt.Errorf("column %d is not in any column family", colID)
for colID, colDesc := range columnIDs {
if !colDesc.Virtual {
if _, ok := colIDToFamilyID[colID]; !ok {
return fmt.Errorf("column %q is not in any column family", colDesc.Name)
}
}
}
return nil
Expand All @@ -2212,7 +2228,9 @@ func (desc *Immutable) validateColumnFamilies(columnIDs map[descpb.ColumnID]stri
// validateCheckConstraints validates that check constraints are well formed.
// Checks include validating the column IDs and verifying that check expressions
// do not reference non-existent columns.
func (desc *Immutable) validateCheckConstraints(columnIDs map[descpb.ColumnID]string) error {
func (desc *Immutable) validateCheckConstraints(
columnIDs map[descpb.ColumnID]*descpb.ColumnDescriptor,
) error {
for _, chk := range desc.AllActiveAndInactiveChecks() {
// Verify that the check's column IDs are valid.
for _, colID := range chk.ColumnIDs {
Expand Down Expand Up @@ -2243,7 +2261,7 @@ func (desc *Immutable) validateCheckConstraints(columnIDs map[descpb.ColumnID]st
// constraints are well formed. Checks include validating the column IDs and
// column names.
func (desc *Immutable) validateUniqueWithoutIndexConstraints(
columnIDs map[descpb.ColumnID]string,
columnIDs map[descpb.ColumnID]*descpb.ColumnDescriptor,
) error {
for _, c := range desc.AllActiveAndInactiveUniqueWithoutIndexConstraints() {
if err := catalog.ValidateName(c.Name, "unique without index constraint"); err != nil {
Expand Down
33 changes: 32 additions & 1 deletion pkg/sql/catalog/tabledesc/structured_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,8 @@ func TestValidateTableDesc(t *testing.T) {

ctx := context.Background()

computedExpr := "1 + 1"

testData := []struct {
err string
desc descpb.TableDescriptor
Expand Down Expand Up @@ -232,6 +234,18 @@ func TestValidateTableDesc(t *testing.T) {
},
NextColumnID: 2,
}},
{`virtual column "virt" is not computed`,
descpb.TableDescriptor{
ID: 2,
ParentID: 1,
Name: "foo",
FormatVersion: descpb.FamilyFormatVersion,
Columns: []descpb.ColumnDescriptor{
{ID: 1, Name: "bar"},
{ID: 2, Name: "virt", Virtual: true},
},
NextColumnID: 3,
}},
{`invalid column ID 0`,
descpb.TableDescriptor{
ID: 2,
Expand Down Expand Up @@ -400,7 +414,7 @@ func TestValidateTableDesc(t *testing.T) {
NextColumnID: 2,
NextFamilyID: 1,
}},
{`column 1 is not in any column family`,
{`column "bar" is not in any column family`,
descpb.TableDescriptor{
ID: 2,
ParentID: 1,
Expand Down Expand Up @@ -431,6 +445,23 @@ func TestValidateTableDesc(t *testing.T) {
NextColumnID: 2,
NextFamilyID: 2,
}},
{`virtual computed column "virt" cannot be part of a family`,
descpb.TableDescriptor{
ID: 2,
ParentID: 1,
Name: "foo",
FormatVersion: descpb.FamilyFormatVersion,
Columns: []descpb.ColumnDescriptor{
{ID: 1, Name: "bar"},
{ID: 2, Name: "virt", ComputeExpr: &computedExpr, Virtual: true},
},
Families: []descpb.ColumnFamilyDescriptor{
{ID: 0, Name: "fam1", ColumnIDs: []descpb.ColumnID{1}, ColumnNames: []string{"bar"}},
{ID: 1, Name: "fam2", ColumnIDs: []descpb.ColumnID{2}, ColumnNames: []string{"virt"}},
},
NextColumnID: 3,
NextFamilyID: 2,
}},
{`table must contain a primary key`,
descpb.TableDescriptor{
ID: 2,
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/catalog/tabledesc/table.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ func MakeColumnDefDescs(
col := &descpb.ColumnDescriptor{
Name: string(d.Name),
Nullable: d.Nullable.Nullability != tree.NotNull && !d.PrimaryKey.IsPrimaryKey,
Virtual: d.IsVirtual(),
}

// Validate and assign column type.
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/catalog/tabledesc/validate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ var validationMap = []struct {
status: todoIAmKnowinglyAddingTechDebt,
reason: "initial import: TODO(features): add validation"},
"ComputeExpr": {status: iSolemnlySwearThisFieldIsValidated},
"Virtual": {status: iSolemnlySwearThisFieldIsValidated},
"PGAttributeNum": {
status: todoIAmKnowinglyAddingTechDebt,
reason: "initial import: TODO(features): add validation"},
Expand Down
18 changes: 16 additions & 2 deletions pkg/sql/create_table.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1399,7 +1399,17 @@ func NewTableDesc(
columnDefaultExprs = append(columnDefaultExprs, nil)
}
if d.IsVirtual() {
return nil, unimplemented.NewWithIssue(57608, "virtual computed columns")
if !sessionData.VirtualColumnsEnabled {
return nil, unimplemented.NewWithIssue(57608, "virtual computed columns")
}
if !evalCtx.Settings.Version.IsActive(ctx, clusterversion.VirtualComputedColumns) {
return nil, pgerror.Newf(pgcode.FeatureNotSupported,
"version %v must be finalized to use virtual columns",
clusterversion.VirtualComputedColumns)
}
if d.HasColumnFamily() {
return nil, pgerror.Newf(pgcode.Syntax, "virtual columns cannot have family specifications")
}
}

col, idx, expr, err := tabledesc.MakeColumnDefDescs(ctx, d, semaCtx, evalCtx)
Expand Down Expand Up @@ -2279,7 +2289,11 @@ func incTelemetryForNewColumn(def *tree.ColumnTableDef, desc *descpb.ColumnDescr
telemetry.Inc(sqltelemetry.SchemaNewTypeCounter(desc.Type.TelemetryName()))
}
if desc.IsComputed() {
telemetry.Inc(sqltelemetry.SchemaNewColumnTypeQualificationCounter("computed"))
if desc.Virtual {
telemetry.Inc(sqltelemetry.SchemaNewColumnTypeQualificationCounter("virtual"))
} else {
telemetry.Inc(sqltelemetry.SchemaNewColumnTypeQualificationCounter("computed"))
}
}
if desc.HasDefault() {
telemetry.Inc(sqltelemetry.SchemaNewColumnTypeQualificationCounter("default_expr"))
Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/exec_util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2188,6 +2188,11 @@ func (m *sessionDataMutator) SetMutliColumnInvertedIndexes(val bool) {
m.data.EnableMultiColumnInvertedIndexes = val
}

// TODO(radu): remove this once the feature is stable.
func (m *sessionDataMutator) SetVirtualColumnsEnabled(val bool) {
m.data.VirtualColumnsEnabled = val
}

// TODO(rytaft): remove this once unique without index constraints are fully
// supported.
func (m *sessionDataMutator) SetUniqueWithoutIndexConstraints(val bool) {
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/pg_catalog
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1894,6 +1894,7 @@ experimental_enable_hash_sharded_indexes off NULL
experimental_enable_multi_column_inverted_indexes off NULL NULL NULL string
experimental_enable_temp_tables off NULL NULL NULL string
experimental_enable_unique_without_index_constraints off NULL NULL NULL string
experimental_enable_virtual_columns off NULL NULL NULL string
extra_float_digits 0 NULL NULL NULL string
force_savepoint_restart off NULL NULL NULL string
foreign_key_cascades_limit 10000 NULL NULL NULL string
Expand Down Expand Up @@ -1967,6 +1968,7 @@ experimental_enable_hash_sharded_indexes off NULL
experimental_enable_multi_column_inverted_indexes off NULL user NULL off off
experimental_enable_temp_tables off NULL user NULL off off
experimental_enable_unique_without_index_constraints off NULL user NULL off off
experimental_enable_virtual_columns off NULL user NULL off off
extra_float_digits 0 NULL user NULL 0 2
force_savepoint_restart off NULL user NULL off off
foreign_key_cascades_limit 10000 NULL user NULL 10000 10000
Expand Down Expand Up @@ -2036,6 +2038,7 @@ experimental_enable_hash_sharded_indexes NULL NULL NULL
experimental_enable_multi_column_inverted_indexes NULL NULL NULL NULL NULL
experimental_enable_temp_tables NULL NULL NULL NULL NULL
experimental_enable_unique_without_index_constraints NULL NULL NULL NULL NULL
experimental_enable_virtual_columns NULL NULL NULL NULL NULL
extra_float_digits NULL NULL NULL NULL NULL
force_savepoint_restart NULL NULL NULL NULL NULL
foreign_key_cascades_limit NULL NULL NULL NULL NULL
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/logictest/testdata/logic_test/show_source
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ experimental_enable_hash_sharded_indexes off
experimental_enable_multi_column_inverted_indexes off
experimental_enable_temp_tables off
experimental_enable_unique_without_index_constraints off
experimental_enable_virtual_columns off
extra_float_digits 0
force_savepoint_restart off
foreign_key_cascades_limit 10000
Expand Down
Loading

0 comments on commit 960b4cf

Please sign in to comment.