Skip to content

Commit

Permalink
sql: add internal function to detect keys in system tables
Browse files Browse the repository at this point in the history
Previously, we had no easy way of knowing if a key belonged
to a system table. This was inadequate because we can improve
our redacted debug zips by including contention information
involving these tables. This patch will add a new builtin
crdb_internal.is_system_table_key which can be used
for conditional redaction in SQL.

Release note: None
  • Loading branch information
fqazi committed Aug 9, 2023
1 parent 4c34b48 commit a4f34e7
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 0 deletions.
5 changes: 5 additions & 0 deletions pkg/sql/faketreeeval/evalctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,11 @@ func (ep *DummyPrivilegedAccessor) LookupZoneConfigByNamespaceID(
return "", false, errors.WithStack(errEvalPrivileged)
}

// IsSystemTable is part of the tree.PrivilegedAccessor interface.
func (ep *DummyPrivilegedAccessor) IsSystemTable(ctx context.Context, id int64) (bool, error) {
return false, errors.WithStack(errEvalPrivileged)
}

// DummySessionAccessor implements the eval.SessionAccessor interface by returning errors.
type DummySessionAccessor struct{}

Expand Down
10 changes: 10 additions & 0 deletions pkg/sql/privileged_accessor.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catid"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/errors"
Expand Down Expand Up @@ -76,6 +77,15 @@ func (p *planner) LookupZoneConfigByNamespaceID(
return tree.DBytes(zc.GetRawBytesInStorage()), true, nil
}

// IsSystemTable implements tree.PrivilegedAccessor.
func (p *planner) IsSystemTable(ctx context.Context, id int64) (bool, error) {
tbl, err := p.Descriptors().ByID(p.Txn()).Get().Table(ctx, catid.DescID(id))
if err != nil {
return false, err
}
return catalog.IsSystemDescriptor(tbl), nil
}

// checkDescriptorPermissions returns nil if the executing user has permissions
// to check the permissions of a descriptor given its ID, or the id given
// is not a descriptor of a table or database.
Expand Down
32 changes: 32 additions & 0 deletions pkg/sql/sem/builtins/builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -5728,6 +5728,38 @@ SELECT
Volatility: volatility.Immutable,
},
),
// Return if a key belongs to a system table, which should make it to print
// within redacted output.
"crdb_internal.is_system_table_key": makeBuiltin(
tree.FunctionProperties{
Category: builtinconstants.CategorySystemInfo,
Undocumented: true,
},
tree.Overload{
Types: tree.ParamTypes{
{Name: "raw_key", Typ: types.Bytes},
},
ReturnType: tree.FixedReturnType(types.Bool),
Fn: func(ctx context.Context, evalCtx *eval.Context, args tree.Datums) (tree.Datum, error) {
_, tableID, err := evalCtx.Codec.DecodeTablePrefix(roachpb.Key(tree.MustBeDBytes(args[0])))
if err != nil {
// If a key isn't prefixed with a table ID ignore.
//nolint:returnerrcheck
return tree.DBoolFalse, nil
}
isSystemTable, err := evalCtx.PrivilegedAccessor.IsSystemTable(ctx, int64(tableID))
if err != nil {
// If we can't find the descriptor or its not the right type then its
// not a system table.
//nolint:returnerrcheck
return tree.DBoolFalse, nil
}
return tree.MakeDBool(tree.DBool(isSystemTable)), nil
},
Info: "This function is used only by CockroachDB's developers for testing purposes.",
Volatility: volatility.Stable,
},
),

// Return a pretty string for a given span, skipping the specified number of
// fields.
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/sem/builtins/fixed_oids.go
Original file line number Diff line number Diff line change
Expand Up @@ -2439,6 +2439,7 @@ var builtinOidsArray = []string{
2466: `crdb_internal.setup_span_configs_stream(tenant_name: string) -> bytes`,
2467: `crdb_internal.request_statement_bundle(stmtFingerprint: string, planGist: string, samplingProbability: float, minExecutionLatency: interval, expiresAfter: interval) -> bool`,
2468: `crdb_internal.request_statement_bundle(stmtFingerprint: string, planGist: string, antiPlanGist: bool, samplingProbability: float, minExecutionLatency: interval, expiresAfter: interval) -> bool`,
2469: `crdb_internal.is_system_table_key(raw_key: bytes) -> bool`,
}

var builtinOidsBySignature map[string]oid.Oid
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/sem/eval/deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,9 @@ type PrivilegedAccessor interface {
// Returns the config byte array, a bool representing whether the namespace exists,
// and an error if there is one.
LookupZoneConfigByNamespaceID(ctx context.Context, id int64) (tree.DBytes, bool, error)

// IsSystemTable returns if a given descriptor ID is a system table.s
IsSystemTable(ctx context.Context, id int64) (bool, error)
}

// RegionOperator gives access to the current region, validation for all
Expand Down

0 comments on commit a4f34e7

Please sign in to comment.