Skip to content

Commit

Permalink
Merge #94339
Browse files Browse the repository at this point in the history 
94339: builtins: implement pg_table_is_visible as UDF r=jordanlewis a=jordanlewis

Now that we support UDFs, we can implement builtin functions as "virtual
UDFs", that are defined in the builtins map.

The builtin appears to be about twice as fast with this method, which is
nice because it gets used a lot in ORM introspection queries.

Release note (performance improvement): improve the performance of
pg_table_is_visible

Epic: None

Co-authored-by: rafiss <[email protected]>

Co-authored-by: Jordan Lewis <[email protected]>
  • Loading branch information
@jordanlewis
craig[bot] and jordanlewis committed Jan 5, 2023
2 parents 4bfbe11 + d363b40 commit d123302
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 86 deletions.
3 changes: 1 addition & 2 deletions pkg/sql/catalog/typedesc/table_implicit_record_type.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,7 @@ func (v TableImplicitRecordType) Adding() bool {

// Dropped implements the Descriptor interface.
func (v TableImplicitRecordType) Dropped() bool {
v.panicNotSupported("Dropped")
return false
return v.desc.Dropped()
}

// Offline implements the Descriptor interface.
Expand Down
21 changes: 20 additions & 1 deletion pkg/sql/pg_catalog.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catprivilege"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descs"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/funcdesc"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/schemadesc"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/schemaexpr"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
Expand Down Expand Up @@ -2601,7 +2602,7 @@ https://www.postgresql.org/docs/9.5/catalog-pg-proc.html`,
return false, err
}

if overload.IsUDF {
if funcdesc.IsOIDUserDefinedFunc(ooid) {
fnDesc, err := p.Descriptors().GetImmutableFunctionByID(
ctx, p.Txn(), descpb.ID(overload.Oid),
tree.ObjectLookupFlags{
Expand All @@ -2618,6 +2619,9 @@ https://www.postgresql.org/docs/9.5/catalog-pg-proc.html`,
if err != nil {
return false, err
}
if fnDesc.Dropped() || fnDesc.GetParentID() != dbContext.GetID() {
return false, nil
}

err = addPgProcUDFRow(h, scDesc, fnDesc, addRow)
if err != nil {
Expand Down Expand Up @@ -3321,6 +3325,10 @@ https://www.postgresql.org/docs/9.5/catalog-pg-type.html`,
return false, err
}

if typDesc.Dropped() {
return false, nil
}

// It's an entry for the implicit record type created on behalf of each
// table. We have special logic for this case.
if typDesc.GetKind() == descpb.TypeDescriptor_TABLE_IMPLICIT_RECORD_TYPE {
Expand All @@ -3333,6 +3341,12 @@ https://www.postgresql.org/docs/9.5/catalog-pg-type.html`,
}
return false, err
}
if !table.IsVirtualTable() && table.GetParentID() != db.GetID() {
// If we're looking an implicit record type for a virtual table, we
// always return the row regardless of the parent DB. But for real
// tables, we only return a row if we're in the same DB as the table.
return false, nil
}
if err := addPGTypeRowForTable(
ctx,
p,
Expand All @@ -3347,6 +3361,11 @@ https://www.postgresql.org/docs/9.5/catalog-pg-type.html`,
return true, nil
}

if typDesc.GetParentID() != db.GetID() {
// Don't return types that aren't in our database.
return false, nil
}

nspOid = schemaOid(sc.GetID())
typ, err = typedesc.HydratedTFromDesc(ctx, tree.NewUnqualifiedTypeName(typDesc.GetName()), typDesc, p)
if err != nil {
Expand Down
107 changes: 24 additions & 83 deletions pkg/sql/sem/builtins/pg_builtins.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1226,26 +1226,14 @@ SELECT description
// https://www.postgresql.org/docs/9.6/static/functions-info.html
"pg_function_is_visible": makeBuiltin(defProps(),
tree.Overload{
IsUDF: true,
Types: tree.ParamTypes{{Name: "oid", Typ: types.Oid}},
ReturnType: tree.FixedReturnType(types.Bool),
Fn: func(ctx context.Context, evalCtx *eval.Context, args tree.Datums) (tree.Datum, error) {
oidArg := tree.MustBeDOid(args[0])
row, err := evalCtx.Planner.QueryRowEx(
ctx, "pg_function_is_visible",
sessiondata.NoSessionDataOverride,
"SELECT n.nspname from pg_proc p JOIN pg_namespace n ON p.pronamespace = n.oid WHERE p.oid=$1 LIMIT 1",
oidArg,
)
if err != nil {
return nil, err
}
if row == nil {
return tree.DNull, nil
}
foundSchemaName := string(tree.MustBeDString(row[0]))
isVisible := evalCtx.SessionData().SearchPath.Contains(foundSchemaName, true /* includeImplicit */)
return tree.MakeDBool(tree.DBool(isVisible)), nil
},
Body: `SELECT n.nspname = any current_schemas(true)
FROM pg_proc p
INNER LOOKUP JOIN pg_namespace n
ON p.pronamespace = n.oid
WHERE p.oid=$1 LIMIT 1`,
Info: "Returns whether the function with the given OID belongs to one of the schemas on the search path.",
Volatility: volatility.Stable,
},
Expand All @@ -1255,26 +1243,14 @@ SELECT description
// https://www.postgresql.org/docs/9.6/static/functions-info.html
"pg_table_is_visible": makeBuiltin(defProps(),
tree.Overload{
IsUDF: true,
Types: tree.ParamTypes{{Name: "oid", Typ: types.Oid}},
ReturnType: tree.FixedReturnType(types.Bool),
Fn: func(ctx context.Context, evalCtx *eval.Context, args tree.Datums) (tree.Datum, error) {
oidArg := tree.MustBeDOid(args[0])
row, err := evalCtx.Planner.QueryRowEx(
ctx, "pg_table_is_visible",
sessiondata.NoSessionDataOverride,
"SELECT n.nspname from pg_class c INNER LOOKUP JOIN pg_namespace n ON c.relnamespace = n.oid WHERE c.oid=$1 LIMIT 1",
oidArg,
)
if err != nil {
return nil, err
}
if row == nil {
return tree.DNull, nil
}
foundSchemaName := string(tree.MustBeDString(row[0]))
isVisible := evalCtx.SessionData().SearchPath.Contains(foundSchemaName, true /* includeImplicit */)
return tree.MakeDBool(tree.DBool(isVisible)), nil
},
Body: `SELECT n.nspname = any current_schemas(true)
FROM pg_class c
INNER LOOKUP JOIN pg_namespace n
ON c.relnamespace = n.oid
WHERE c.oid=$1 LIMIT 1`,
Info: "Returns whether the table with the given OID belongs to one of the schemas on the search path.",
Volatility: volatility.Stable,
},
Expand All @@ -1288,26 +1264,14 @@ SELECT description
// https://www.postgresql.org/docs/9.6/static/functions-info.html
"pg_type_is_visible": makeBuiltin(defProps(),
tree.Overload{
IsUDF: true,
Types: tree.ParamTypes{{Name: "oid", Typ: types.Oid}},
ReturnType: tree.FixedReturnType(types.Bool),
Fn: func(ctx context.Context, evalCtx *eval.Context, args tree.Datums) (tree.Datum, error) {
oidArg := tree.MustBeDOid(args[0])
row, err := evalCtx.Planner.QueryRowEx(
ctx, "pg_type_is_visible",
sessiondata.NoSessionDataOverride,
"SELECT n.nspname from pg_type t JOIN pg_namespace n ON t.typnamespace = n.oid WHERE t.oid=$1 LIMIT 1",
oidArg,
)
if err != nil {
return nil, err
}
if row == nil {
return tree.DNull, nil
}
foundSchemaName := string(tree.MustBeDString(row[0]))
isVisible := evalCtx.SessionData().SearchPath.Contains(foundSchemaName, true /* includeImplicit */)
return tree.MakeDBool(tree.DBool(isVisible)), nil
},
Body: `SELECT n.nspname = any current_schemas(true)
FROM pg_type t
INNER LOOKUP JOIN pg_namespace n
ON t.typnamespace = n.oid
WHERE t.oid=$1 LIMIT 1`,
Info: "Returns whether the type with the given OID belongs to one of the schemas on the search path.",
Volatility: volatility.Stable,
},
Expand Down Expand Up @@ -2092,43 +2056,20 @@ SELECT description
// _pg_index_position return the column's position in the index
// (or NULL if not there).
//
// NOTE: this could be defined as a user-defined function, like
// it is in Postgres:
// NOTE: this is defined as a UDF, same as in Postgres:
// https://github.com/postgres/postgres/blob/master/src/backend/catalog/information_schema.sql
//
// CREATE FUNCTION _pg_index_position(oid, smallint) RETURNS int
// LANGUAGE sql STRICT STABLE
// BEGIN ATOMIC
// SELECT (ss.a).n FROM
// (SELECT information_schema._pg_expandarray(indkey) AS a
// FROM pg_catalog.pg_index WHERE indexrelid = $1) ss
// WHERE (ss.a).x = $2;
// END;
//
"information_schema._pg_index_position": makeBuiltin(defProps(),
tree.Overload{
IsUDF: true,
Types: tree.ParamTypes{
{Name: "oid", Typ: types.Oid},
{Name: "col", Typ: types.Int2},
},
ReturnType: tree.FixedReturnType(types.Int),
Fn: func(ctx context.Context, evalCtx *eval.Context, args tree.Datums) (tree.Datum, error) {
r, err := evalCtx.Planner.QueryRowEx(
ctx, "information_schema._pg_index_position",
sessiondata.NoSessionDataOverride,
`SELECT (ss.a).n FROM
(SELECT information_schema._pg_expandarray(indkey) AS a
FROM pg_catalog.pg_index WHERE indexrelid = $1) ss
WHERE (ss.a).x = $2`,
args[0], args[1])
if err != nil {
return nil, err
}
if len(r) == 0 {
return tree.DNull, nil
}
return r[0], nil
},
Body: `SELECT (ss.a).n FROM
(SELECT information_schema._pg_expandarray(indkey) AS a
FROM pg_catalog.pg_index WHERE indexrelid = $1) ss
WHERE (ss.a).x = $2`,
Info: notUsableInfo,
Volatility: volatility.Stable,
},
Expand Down

0 comments on commit d123302

Please sign in to comment.