sql: add missing obj_description case #88098
Merged
+115
−3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
|
@rafiss do you happen to know where obj_description is being tested? I can't find the tests. |
knz
force-pushed
the
20220917-sc-comments
branch
from
c206b29 to
d2a7bb0
Compare
15 tasks
rafiss
reviewed
Sep 18, 2022
There was a problem hiding this comment.
thanks for this fix! there are tests in the following places:
pkg/sql/comment_on_table_test.go
pkg/sql/comment_on_database_test.go
pkg/sql/comment_on_index_test.go
pkg/sql/logictest/testdata/logic_test/privileges_comments
Reviewable status:
complete! 0 of 0 LGTMs obtained
knz
force-pushed
the
20220917-sc-comments
branch
from
d2a7bb0 to
f068697
Compare
|
Thanks! |
rafiss
approved these changes
Sep 20, 2022
There was a problem hiding this comment.
Reviewable status:
pkg/sql/comment_on_schema_test.go line 107 at r2 (raw file):
} t.Fatal("dropped comment remain comment")
nit: should it say "dropped schema"?
Found with the following query: ```sql COMMENT ON SCHEMA public IS 'hello'; SELECT obj_description(oid, 'pg_namespace') FROM pg_namespace WHERE nspname = 'public'; ``` Release note (sql change): The PostgreSQL compatibility function `obj_description` now supports retrieving comments on schemas.
knz
force-pushed
the
20220917-sc-comments
branch
from
f068697 to
3e22bb1
Compare
knz
commented
Sep 20, 2022
There was a problem hiding this comment.
Reviewable status:
pkg/sql/comment_on_schema_test.go line 107 at r2 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
nit: should it say "dropped schema"?
Sure. But I had copy-pasted this from the other test files, so I'm fixing those too now.
|
bors r=rafiss |
|
Build failed (retrying...): |
|
Build succeeded: |
This was referenced Sep 20, 2022
This was referenced Sep 20, 2022
craig bot
pushed a commit
that referenced
this pull request
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Needed for #88061.
Found bug with the following query:
Release note (sql change): The PostgreSQL compatibility function
obj_descriptionnow supports retrieving comments on schemas.Fixes #95322.