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 fails for UDFs #94953

Closed
knz opened this issue Jan 9, 2023 · 6 comments · Fixed by #94771
Closed

sql: pg_function_is_visible fails for UDFs #94953

knz opened this issue Jan 9, 2023 · 6 comments · Fixed by #94771
Assignees
Labels
A-schema-catalog Related to the schema descriptors collection and the catalog API in general. A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-routine UDFs and Stored Procedures A-sql-vtables Virtual tables - pg_catalog, information_schema etc 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

@knz
Copy link
Contributor

knz commented Jan 9, 2023

Found while working on #88061.

Describe the problem

pg_function_is_visible() fails for the OID of UDFs.

To Reproduce

> CREATE FUNCTION myfunc(val INT) RETURNS INT CALLED ON NULL INPUT LANGUAGE SQL AS $$ SELECT val $$;
> select pg_function_is_visible(oid) from pg_proc where proname='myfunc';
ERROR: function 100105 does not exist: function undefined
SQLSTATE: 42883

Expected behavior

All the OID values in pg_proc are valid inputs to pg_function_is_visible.

cc @rafiss @mgartner @chengxiong-ruan for triage.

Jira issue: CRDB-23241
Epic: CRDB-23454

@knz knz 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-vtables Virtual tables - pg_catalog, information_schema etc A-schema-catalog Related to the schema descriptors collection and the catalog API in general. labels Jan 9, 2023
@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Jan 9, 2023
@knz knz changed the title sql: pg_function_is_visible fails for some built-in functions sql: pg_function_is_visible fails for UDFs Jan 9, 2023
@rafiss
Copy link
Collaborator

rafiss commented Jan 9, 2023

i think this one will be fixed by #94771

@mgartner
Copy link
Collaborator

mgartner commented Jan 9, 2023

This looks like a regression introduced in acf5006. It works on v22.2.1:

defaultdb> select version();
                                        version
----------------------------------------------------------------------------------------
  CockroachDB CCL v22.2.1 (x86_64-apple-darwin19, built 2022/12/22 15:14:07, go1.19.1)
(1 row)


defaultdb> CREATE FUNCTION myfunc(val INT) RETURNS INT CALLED ON NULL INPUT LANGUAGE SQL AS $$ SELECT val $$;
CREATE FUNCTION

defaultdb> select pg_function_is_visible(oid) from pg_proc where proname='myfunc';
  pg_function_is_visible
--------------------------
            t
(1 row)

@mgartner mgartner added the A-sql-routine UDFs and Stored Procedures label Jan 9, 2023
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Jan 9, 2023
@rafiss
Copy link
Collaborator

rafiss commented Jan 9, 2023

I think the regression is in cee3ee9, not acf5006.

@mgartner
Copy link
Collaborator

mgartner commented Jan 9, 2023

Ahh, good catch!

@jordanlewis
Copy link
Member

Yeah I happened to notice this as well, it's fixed here: 7221769

Will get this merged soon;

@rafiss
Copy link
Collaborator

rafiss commented Jan 9, 2023

also note that v22.2 has a different problem with pg_function_is_visible for UDFs. It's too permissive, since it doesn't actually inspect the search_path. See #89546

Backporting the fixes that we've made on master isn't feasible since it would require other changes, but I can make a smaller change just for v22.2.

@exalate-issue-sync exalate-issue-sync bot removed the T-sql-queries SQL Queries Team label Jan 10, 2023
@craig craig bot closed this as completed in 04762b7 Jan 15, 2023
craig bot pushed a commit that referenced this issue Feb 24, 2023
88061: clisqlshell: new infrastructure for describe commands r=rafiss,ZhouXing19 a=knz

Fixes #95320.
Epic: CRDB-23454

The SQL shell (`cockroach sql`, `demo`) now
supports the client-side commands `\l`, `\dn`, `\d`, `\di`, `\dm`,
`\ds`, `\dt`, `\dv`, `\dC`, `\dT`, `\dd`, `\dg`, `\du`, `\df` and `\dd` in a
way similar to `psql`, including the modifier flags `S` and `+`, for
convenience for users migrating from PostgreSQL.

A notable difference is that when a pattern argument is specified, it
should use the SQL "LIKE" syntax (with `%` representing the wildcard
character) instead of PostgreSQL's glob-like syntax (with `*`
representing wildcards).

Issues discovered:

- [x] join bug:  #88096
- [x] semi-join exec error #91012
- [x] `pg_table_is_visible` should return true when given a valid index OID and the index is valid.  #88097
- [x] missing pkey column in pg_index:  #88106
- [x] missing stored columns in pg_index: #88107 
- [x] pg_statistic_ext has problems #88108
- [x] missing view def on materialized views  #88109
- [x] missing schema comments: #88098
- [x] missing pronamespace for functions #94952
- [x] broken pg_function_is_visible for UDFs #94953
- [x] generated columns #92545
- [x] indnullsnotdistinct #92583
- [x] missing prokind #95288
- [x] missing function comments in obj_description #95292
- [x] planning regression #95633

96397: builtins: mark some pg_.* builtins as strict r=DrewKimball a=mgartner

Builtins defined using the UDF `Body` field will be wrapped in a `CASE`
expression if they are strict, i.e., `CalledOnNullInput=false`. When the
builtin is inlined, the `CASE` expression prevents decorrelation,
leaving a slow apply-join in the query plan. This caused a significant
regression of some ORM introspection queries.

Some of these builtins have filters that cause the SQL body to return no rows
if any of the arguments is NULL. In this case, the builtin will have the same
behavior whether or not it is defined as being strict. We can safely optimize
these builtins by setting `CalledOnNullInput=true`.

The following conditions are sufficient to prove that `CalledOnNullInput` can
be set for a builtin function with a SQL body:

  1. The WHERE clause of the SQL query *null-rejects* every argument of the
     builtin. Operators like `=` and `<` *null-reject* their operands because
     they filter rows for which an operand is NULL.

  2. The arguments are not used elsewhere in the query. This is not strictly
     necessary, but simplifies the proof because it ensures NULL arguments will
     not cause the builtin to error.

Examples of SQL statements that would allow `CalledOnNullInput` to be set:
```
SELECT * FROM tab WHERE $1=1 AND $2='two';

SELECT * FROM tab WHERE $1 > 0;
```

Fixes #96218
Fixes #95569

Epic: None

Release note: None


97585: cli: don't scope TLS client certs to a specific tenant by default r=stevendanna a=knz

Epic: CRDB-23559
Fixes: #97584

This commit changes the default for `--tenant-scope` from "only the system tenant" to "cert valid for all tenants".

Note that the scoping is generally useful for security, and it is used in CockroachCloud. However, CockroachCloud does not use our CLI code to generate certs and sets its cert tenant scopes on its own.

Given that our CLI code is provided for convenience and developer productivity, and we don't expect certs generated here to be used in multi-tenant deployments where tenants are adversarial to each other, defaulting to certs that are valid on every tenant is a good choice.

Release note: None


Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-schema-catalog Related to the schema descriptors collection and the catalog API in general. A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-routine UDFs and Stored Procedures A-sql-vtables Virtual tables - pg_catalog, information_schema etc 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
None yet
4 participants