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: index creation fails if issued just after create table in same txn #13505

Closed
novabyte opened this issue Feb 9, 2017 · 6 comments · Fixed by #15929
Closed

sql: index creation fails if issued just after create table in same txn #13505

novabyte opened this issue Feb 9, 2017 · 6 comments · Fixed by #15929
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-support Would prevent or help troubleshoot a customer escalation - bugs, missing observability/tooling, docs S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.
Milestone

Comments

@novabyte
Copy link

novabyte commented Feb 9, 2017

The latest Nakama server schema fails to be created when run as a single transaction.

To reproduce:

  1. Run CREATE DATABASE nakama;
  2. Add BEGIN; and COMMIT; around the SQL file and execute:
cockroach sql --url "postgresql://[email protected]:26257/nakama?sslmode=disable" < ./20170115200001_initial_schema.sql

The schema creation will fail with:

BEGIN
CREATE TABLE
CREATE TABLE
pq: table is being added
Error: pq: table is being added
Failed running "sql"

If this line from the schema is removed:

CREATE INDEX IF NOT EXISTS user_id_idx ON user_device (user_id);

The schema creation will complete successfully. This may be an issue with the schema but it works without modification when run with notransaction.

This is tested against cockroachdb configuration:

$> cockroach start --insecure --store=attrs=ssd,path=/tmp/cockroach
CockroachDB node starting at 2017-02-09 07:06:39.16693333 +0000 GMT
build:      CCL beta-20170126 @ 2017/02/09 07:00:11 (go1.7.5)
admin:      http://localhost.local:8080
sql:        postgresql://[email protected]:26257?sslmode=disable
logs:       /tmp/cockroach/logs
store[0]:   path=/tmp/cockroach,attrs=ssd
status:     initialized new cluster
clusterID:  66b8f5e6-d707-41d5-b835-9f7c13d93508
nodeID:     1
@novabyte novabyte changed the title Schema creation fails when index (with foreign key?) added to table in transaction Schema creation fails when index added to table (with foreign key?) in transaction Feb 9, 2017
@knz knz added the S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption. label Feb 9, 2017
@knz knz changed the title Schema creation fails when index added to table (with foreign key?) in transaction sql: index creation fails if issued just after create table in same txn Feb 9, 2017
@knz knz added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Feb 9, 2017
@knz
Copy link
Contributor

knz commented Feb 9, 2017

cc @dt @vivekmenezes

@knz knz added this to the 1.0 milestone Feb 9, 2017
@knz
Copy link
Contributor

knz commented Feb 9, 2017

extra comment from gitter:

(14:41) <@novabyte> @knz I think I know where the issue is which I reported. "Have CockroachDB automatically create an index of the foreign key columns for you. However, it’s important to
note that if you later remove the Foreign Key constraint, this automatically created index is not removed." (From the Foreign Key docs)
(14:41) <@novabyte> Perhaps it gets confused because I try and index a column with a foreign key which will be implicitly indexed by cockroachdb?

@dt
Copy link
Member

dt commented Feb 9, 2017

I'm guessing this is because we need to use the intermediate non-public state when creating tables that have dependencies on other tables, and then make them public only after we waitForOneVersion and propagate the dependency information into the other tables... that process can't start until the creation (in said non-public state) commits, so until commit (or 2s after commit to be pedantic), the table is still in the funny non-public state.

@novabyte
Copy link
Author

novabyte commented Feb 9, 2017

I've read the documentation for foreign keys in more detail and experimented with the creation of the schema in a single transaction and without. The single transaction (if I don't add the index called user_id_idx) creates:

root@:26257> SHOW INDEXES FROM user_device;
+-------------+---------------------------------------------+--------+-----+---------+-----------+---------+----------+
|    Table    |                    Name                     | Unique | Seq | Column  | Direction | Storing | Implicit |
+-------------+---------------------------------------------+--------+-----+---------+-----------+---------+----------+
| user_device | primary                                     | true   |   1 | id      | ASC       | false   | false    |
| user_device | user_device_auto_index_fk_user_id_ref_users | false  |   1 | user_id | ASC       | false   | false    |
| user_device | user_device_auto_index_fk_user_id_ref_users | false  |   2 | id      | ASC       | false   | true     |
+-------------+---------------------------------------------+--------+-----+---------+-----------+---------+----------+

The non-transactional schema when applied creates:

root@:26257> SHOW INDEXES FROM user_device;
+-------------+---------------------------------------------+--------+-----+---------+-----------+---------+----------+
|    Table    |                    Name                     | Unique | Seq | Column  | Direction | Storing | Implicit |
+-------------+---------------------------------------------+--------+-----+---------+-----------+---------+----------+
| user_device | primary                                     | true   |   1 | id      | ASC       | false   | false    |
| user_device | user_device_auto_index_fk_user_id_ref_users | false  |   1 | user_id | ASC       | false   | false    |
| user_device | user_device_auto_index_fk_user_id_ref_users | false  |   2 | id      | ASC       | false   | true     |
| user_device | user_id_idx                                 | false  |   1 | user_id | ASC       | false   | false    |
| user_device | user_id_idx                                 | false  |   2 | id      | ASC       | false   | true     |
+-------------+---------------------------------------------+--------+-----+---------+-----------+---------+----------+

Both user_device_auto_index_fk_user_id_ref_users and user_id_idx appear to have the exact same index properties. Does this mean those indexes are identical on disk with different names?

@dt
Copy link
Member

dt commented Feb 9, 2017

Ah, yep, the separate CREATE INDEX user_id_idx statement is after the CREATE TABLE, and thus does indeed create an identical, duplicative index to the one the CREATE TABLE auto-created for the FK -- CREATE TABLE looked though the indexes to see if any matched for the FK, didn't find one (because the CREATE INDEX hasn't been run yet) and thus automatically added it.

FWIW, if the index were included in the CREATE TABLE user_device statement instead, I think the FK would use it, e.g.

CREATE TABLE IF NOT EXISTS user_device (
    PRIMARY KEY (id),
    INDEX user_id_idx (user_id),
    FOREIGN KEY (user_id) REFERENCES users(id),
    id      VARCHAR(64) NOT NULL,
    user_id BYTEA       NOT NULL
);

@vivekmenezes vivekmenezes modified the milestones: Later, 1.0 Feb 26, 2017
@dianasaur323 dianasaur323 added O-support Would prevent or help troubleshoot a customer escalation - bugs, missing observability/tooling, docs and removed community-questions labels Mar 14, 2017
@vivekmenezes
Copy link
Contributor

after #14368 we disallow statements that follow a certain type of schema change in the same transaction. This restriction applies to schema changes that are rolled out in many transactions. CREATE TABLE on its own without the use of FKs can be followed by another statement in the same transaction because it is a schema change that can be fully executed within the same transaction.

vivekmenezes added a commit to vivekmenezes/cockroach that referenced this issue May 17, 2017
A new table descriptor is placed in the non-public TableDescriptor_ADD state
when it refers to another table through an FK or interleaved reference,
and can only be made public once the other tables back reference is
visible across the cluster.

This change allows schema change operations on such non-public tables,
while preserving the non-public behavior for table leases.

fixes cockroachdb#13505
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-support Would prevent or help troubleshoot a customer escalation - bugs, missing observability/tooling, docs S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants