Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: pg_function_is_visible is not correct for UDFs #89546

Closed
rafiss opened this issue Oct 6, 2022 · 1 comment · Fixed by #90657
Closed

sql: pg_function_is_visible is not correct for UDFs #89546

rafiss opened this issue Oct 6, 2022 · 1 comment · Fixed by #90657
Assignees
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-routine UDFs and Stored Procedures C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@rafiss
Copy link
Collaborator

rafiss commented Oct 6, 2022

Describe the problem

This builtin was added in 2019. The assumptions from back then no longer hold. See:

// pg_function_is_visible returns true if the input oid corresponds to a
// builtin function that is part of the databases on the search path.
// CockroachDB doesn't have a concept of namespaced functions, so this is
// always true if the builtin exists at all, and NULL otherwise.
// https://www.postgresql.org/docs/9.6/static/functions-info.html
"pg_function_is_visible": makeBuiltin(defProps(),
tree.Overload{
Types: tree.ArgTypes{{"oid", types.Oid}},
ReturnType: tree.FixedReturnType(types.Bool),
Fn: func(ctx *eval.Context, args tree.Datums) (tree.Datum, error) {
oid := tree.MustBeDOid(args[0])
t, err := ctx.Planner.QueryRowEx(
ctx.Ctx(), "pg_function_is_visible",
sessiondata.NoSessionDataOverride,
"SELECT * from pg_proc WHERE oid=$1 LIMIT 1", oid.Oid)
if err != nil {
return nil, err
}

Expected behavior
Match the Postgres implementation, and take into account the schemas on the search path. Maybe it also takes into account privileges?

Also, the current implementation is needlessly slow. I hope we can improve the speed of it too.

Environment:

  • CockroachDB v22.2.0-beta

cc @chengxiong-ruan @mgartner

Jira issue: CRDB-20297

Epic CRDB-19147

@rafiss rafiss added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-routine UDFs and Stored Procedures labels Oct 6, 2022
@blathers-crl blathers-crl bot added T-sql-schema-deprecated Use T-sql-foundations instead T-sql-queries SQL Queries Team labels Oct 6, 2022
@rafiss
Copy link
Collaborator Author

rafiss commented Oct 25, 2022

The fix here is going to be every similar to what I did in #90649 so I can take this issue.

@rafiss rafiss self-assigned this Oct 25, 2022
@craig craig bot closed this as completed in 16c786f Dec 13, 2022
@exalate-issue-sync exalate-issue-sync bot removed the T-sql-queries SQL Queries Team label Dec 13, 2022
@exalate-issue-sync exalate-issue-sync bot added T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) and removed T-sql-schema-deprecated Use T-sql-foundations instead labels May 10, 2023
@mgartner mgartner moved this to Done in SQL Queries Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-routine UDFs and Stored Procedures C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant