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: fill information_schema.columns.is_identity correctly #61011

Closed
arulajmani opened this issue Feb 23, 2021 · 2 comments · Fixed by #61348
Closed

sql: fill information_schema.columns.is_identity correctly #61011

arulajmani opened this issue Feb 23, 2021 · 2 comments · Fixed by #61348
Assignees
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-sequences Sequence handling in SQL A-tools-prisma C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@arulajmani
Copy link
Collaborator

Describe the problem

Currently, information_schema.columns.is_identity is hard-coded as DNull. Instead, we should fill it correctly with either "yes" or "no" .

@arulajmani arulajmani 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-sequences Sequence handling in SQL labels Feb 23, 2021
@rafiss
Copy link
Collaborator

rafiss commented Feb 26, 2021

I think based on these docs, this column might specifically refer to identity columns as created by this syntax: #48532 (which we don't support yet). In which case, maybe we just always return false? We should confirm.

https://www.postgresql.org/docs/current/infoschema-columns.html

@rafiss
Copy link
Collaborator

rafiss commented Mar 1, 2021

Confirmed that in PostgreSQL, only GENERATED .. AS IDENTITY columns show true for this:

rafiss@127:postgres> create table ts (a serial primary key, b int);
CREATE TABLE

rafiss@127:postgres> create table t_identity (a int generated always as identity, b int);
CREATE TABLE

rafiss@127:postgres> select column_name, is_identity from information_schema.columns where table_name = 'ts';
+---------------+---------------+
| column_name   | is_identity   |
|---------------+---------------|
| a             | NO            |
| b             | NO            |
+---------------+---------------+

rafiss@127:postgres> select column_name, is_identity from information_schema.columns where table_name = 't_identity';
+---------------+---------------+
| column_name   | is_identity   |
|---------------+---------------|
| a             | YES           |
| b             | NO            |
+---------------+---------------+

So for now, we should always return false in CockroachDB. We should leave a TODO comment in the code with a link to #48532

craig bot pushed a commit that referenced this issue Mar 3, 2021
59339: sql: audit all usages of Query to use iterator pattern r=yuzefovich a=yuzefovich

Similarly to the previous commit (dbc8676), here we audit all usages of `Query`
method of the internal executor to take advantage of the iterator API
wherever possible (or switching to `Exec` or `QueryRow`).
`QueryBuffered` has been added to the interface too.

The only place where it would be beneficial to use the iterator pattern
but it is not done currently is for `SHOW STATISTICS` statement - in
there, we have a panic-catcher which works only on the assumption of not
updating any of the shared state (which the iterator API contradicts).
Refactoring that part is left as a TODO.

Fixes: #48595.

Release justification: low-risk update to existing functionality.

Release note: None

60283: sql: improved diff tool for any namespace r=rafiss a=mnovelodou

Previously, diff tool worked only for pg_catalog
This was inadequate because it can be used for information_schema as well
To address this, this patch takes the namespace as parameter to compare a
different database

Release note: None
Release justification: non-production code changes

Fixes #58037

61263: roachtest: mark pgx prepared stmt test as passing r=otan a=rafiss

fixes #61250 

Release justification: testing only change

Release note: None

61304: ccl: test that local scan is planned for RBR table with computed region r=rytaft a=rytaft

This commit adds a test that a local scan is planned for a `REGIONAL BY ROW`
table with a computed region column.

Informs #57722

Release justification: non-production code changes.

Release note: None

61321: sql: fix tracing of postqueries r=yuzefovich a=yuzefovich

When we're performing tracing, we derive a special `consumeCtx` when
creating a DistSQLReceiver for the whole plan. That context should be
used for all components of the physical plan (main query, sub- and
post-queries), and the span needs to be finished by calling the stored
`cleanup` function. Previously, this was called in `ProducerDone` of the
main query which resulted in the span being finished before the
post-queries are run. As a result, the tracing spans for cascades and
checks could be incomplete.

This commit fixes the problem by delaying the finish of the span until
the DistSQLReceiver is released (since it is a convenient place that all
callers of `MakeDistSQLReceiver` call in a deferred invocation).

Release justification: bug fix.

Release note (bug fix): Previously, the traces of cascades and checks
could be incomplete, and now it is fixed.

61348: sql: fixing information_schema.columns.is_identity r=rafiss a=mnovelodou

Previously, information_schema.columns.is_identity was set to null
This was inadequate because is incorrect value
To address this, this patch sets column to NO as identity columns
are not yet supported

Release justification: bug fixes and low-risk updates to new functionality
Release note (sql change): fixed information_schema.columns.is_identity to
display the correct value

Fixes #61011

61375: kvserver: add a log.Scope r=andreimatei a=andreimatei

Release note: None
Release justification: Test only.

Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: MiguelNovelo <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Rebecca Taft <[email protected]>
Co-authored-by: Andrei Matei <[email protected]>
@craig craig bot closed this as completed in 0ae5cb7 Mar 3, 2021
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-sequences Sequence handling in SQL A-tools-prisma C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants