Skip to content

Commit

Permalink
sql/resolver: allow ResolveMutableExistingTableObject to include offl…
Browse files Browse the repository at this point in the history
…ine tables

This patch is required for cockroachdb#138297, where an LDR stream is created from a
source that is offline. The subsequent PR will add test coverage to this.

Epic: none

Release note: none
  • Loading branch information
msbutler committed Jan 9, 2025
1 parent 31e5601 commit 1a0582b
Show file tree
Hide file tree
Showing 9 changed files with 14 additions and 11 deletions.
4 changes: 2 additions & 2 deletions pkg/crosscluster/logical/create_logical_replication_stmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ func resolveDestinationObjects(
} else if resolved.ParentSchemaID != resPrefix.Schema.GetID() {
return resolved, errors.Newf("destination tables must all be in the same schema")
}
if _, _, err := resolver.ResolveMutableExistingTableObject(ctx, r, &dstTableName, true, tree.ResolveRequireTableDesc); err == nil {
if _, _, err := resolver.ResolveMutableExistingTableObject(ctx, r, &dstTableName, true, tree.ResolveRequireTableDesc, false /* includeOffline */); err == nil {
return resolved, errors.Newf("destination table %s already exists", destResources.Tables[i])
}
tbNameWithSchema := tree.MakeTableNameWithSchema(
Expand All @@ -341,7 +341,7 @@ func resolveDestinationObjects(
)
resolved.TableNames = append(resolved.TableNames, tbNameWithSchema)
} else {
prefix, td, err := resolver.ResolveMutableExistingTableObject(ctx, r, &dstTableName, true, tree.ResolveRequireTableDesc)
prefix, td, err := resolver.ResolveMutableExistingTableObject(ctx, r, &dstTableName, true, tree.ResolveRequireTableDesc, false /* includeOffline */)
if err != nil {
return resolved, errors.Wrapf(err, "failed to find existing destination table %s", destResources.Tables[i])
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/crosscluster/producer/replication_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (r *replicationStreamManagerImpl) StartReplicationStreamForTables(
mutableTableDescs := make([]*tabledesc.Mutable, 0, len(req.TableNames))
tableIDs := make([]uint32, 0, len(req.TableNames))

externalCatalog, err := externalcatalog.ExtractExternalCatalog(ctx, r.resolver, r.txn, r.txn.Descriptors(), req.TableNames...)
externalCatalog, err := externalcatalog.ExtractExternalCatalog(ctx, r.resolver, r.txn, r.txn.Descriptors(), false /* includeOffline */, req.TableNames...)
if err != nil {
return streampb.ReplicationProducerSpec{}, err
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/sql/catalog/externalcatalog/external_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func ExtractExternalCatalog(
schemaResolver resolver.SchemaResolver,
txn isql.Txn,
descCol *descs.Collection,
includeOffline bool,
tableNames ...string,
) (externalpb.ExternalCatalog, error) {
externalCatalog := externalpb.ExternalCatalog{}
Expand All @@ -48,7 +49,7 @@ func ExtractExternalCatalog(
return externalpb.ExternalCatalog{}, err
}
tn := uon.ToTableName()
_, td, err := resolver.ResolveMutableExistingTableObject(ctx, schemaResolver, &tn, true, tree.ResolveRequireTableDesc)
_, td, err := resolver.ResolveMutableExistingTableObject(ctx, schemaResolver, &tn, true, tree.ResolveRequireTableDesc, includeOffline)
if err != nil {
return externalpb.ExternalCatalog{}, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/catalog/externalcatalog/external_catalog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func TestExtractIngestExternalCatalog(t *testing.T) {
)
defer close()

catalog, err = ExtractExternalCatalog(ctx, planner.(resolver.SchemaResolver), txn, col, tableNames...)
catalog, err = ExtractExternalCatalog(ctx, planner.(resolver.SchemaResolver), txn, col, false, tableNames...)
return err
})
return catalog, err
Expand Down
2 changes: 2 additions & 0 deletions pkg/sql/catalog/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,14 @@ func ResolveMutableExistingTableObject(
tn *tree.TableName,
required bool,
requiredType tree.RequiredTableKind,
includeOffline bool,
) (prefix catalog.ResolvedObjectPrefix, res *tabledesc.Mutable, err error) {
lookupFlags := tree.ObjectLookupFlags{
Required: required,
RequireMutable: true,
DesiredObjectKind: tree.TableObject,
DesiredTableDescKind: requiredType,
IncludeOffline: includeOffline,
}
// TODO: As part of work for #34240, an UnresolvedObjectName should be
// passed as an argument to this function.
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/create_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,7 @@ func ResolveFK(
originCols[i] = col
}

_, target, err := resolver.ResolveMutableExistingTableObject(ctx, sc, &d.Table, true /*required*/, tree.ResolveRequireTableDesc)
_, target, err := resolver.ResolveMutableExistingTableObject(ctx, sc, &d.Table, true /*required*/, tree.ResolveRequireTableDesc, false /* includeOffline */)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/region_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -2498,7 +2498,7 @@ func (p *planner) optimizeSystemDatabase(ctx context.Context) error {
)
required := true
_, desc, err := resolver.ResolveMutableExistingTableObject(
ctx, p, &tableName, required, tree.ResolveRequireTableDesc)
ctx, p, &tableName, required, tree.ResolveRequireTableDesc, false /* includeOffline */)
return desc, err
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ type resolveFlags struct {
func (p *planner) ResolveMutableTableDescriptor(
ctx context.Context, tn *tree.TableName, required bool, requiredType tree.RequiredTableKind,
) (prefix catalog.ResolvedObjectPrefix, table *tabledesc.Mutable, err error) {
prefix, desc, err := resolver.ResolveMutableExistingTableObject(ctx, p, tn, required, requiredType)
prefix, desc, err := resolver.ResolveMutableExistingTableObject(ctx, p, tn, required, requiredType, false /* includeOffline */)
if err != nil {
return prefix, nil, err
}
Expand Down Expand Up @@ -653,7 +653,7 @@ func expandIndexName(
tn = &index.Table
if tn.Object() != "" {
// The index and its table prefix must exist already. Resolve the table.
_, desc, err = resolver.ResolveMutableExistingTableObject(ctx, p, tn, requireTable, tree.ResolveRequireTableOrViewDesc)
_, desc, err = resolver.ResolveMutableExistingTableObject(ctx, p, tn, requireTable, tree.ResolveRequireTableOrViewDesc, false /* includeOffline */)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -1139,7 +1139,7 @@ func (p *planner) ResolveMutableTableDescriptorEx(
requiredType tree.RequiredTableKind,
) (catalog.ResolvedObjectPrefix, *tabledesc.Mutable, error) {
tn := name.ToTableName()
prefix, table, err := resolver.ResolveMutableExistingTableObject(ctx, p, &tn, required, requiredType)
prefix, table, err := resolver.ResolveMutableExistingTableObject(ctx, p, &tn, required, requiredType, false /* includeOffline */)
if err != nil {
return catalog.ResolvedObjectPrefix{}, nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/sequence.go
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,7 @@ func GetSequenceDescFromIdentifier(
}
} else {
// This is only executed via IMPORT which uses its own resolver.
_, seqDesc, err = resolver.ResolveMutableExistingTableObject(ctx, sc, &tn, true /*required*/, tree.ResolveRequireSequenceDesc)
_, seqDesc, err = resolver.ResolveMutableExistingTableObject(ctx, sc, &tn, true /*required*/, tree.ResolveRequireSequenceDesc, false /*requiredType*/)
if err != nil {
return nil, err
}
Expand Down

0 comments on commit 1a0582b

Please sign in to comment.