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.

Sign up for GitHub

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

Jump to bottom

sql: automatically add the needed index during alter table add fk constraint on empty tables #26738

Closed
BramGruneir opened this issue Jun 14, 2018 · 8 comments
Closed

sql: automatically add the needed index during alter table add fk constraint on empty tables #26738

BramGruneir opened this issue Jun 14, 2018 · 8 comments
Assignees
@BramGruneir @thoszhang
Labels
A-schema-changes A-tools-hibernate Issues that pertain to Hibernate integration. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) E-easy Easy issue to tackle, requires little or no CockroachDB experience

Comments

@BramGruneir
Copy link
Member

This was motivated by hibernate, which for every test creates all of the tables it needs, then adds all the constraints. And it dumps a stack trace and warning when adding the fk constraints.

Right now, we only auto add the index if the alter table is within the same transaction or part of the create table statement. This proposes that we allow the auto addition of the index if the table is empty as well.

cc: @dt, @vivekmenezes
@BramGruneir BramGruneir added the A-tools-hibernate Issues that pertain to Hibernate integration. label Jun 14, 2018
@BramGruneir BramGruneir self-assigned this Jun 14, 2018
@BramGruneir BramGruneir changed the title sql: automatically add the needed index during alter table add foreign key constraint sql: automatically add the needed index during alter table add fk constraint on empty tables Jun 14, 2018
@BramGruneir BramGruneir mentioned this issue Jun 14, 2018
Closed
14 tasks
@BramGruneir
Copy link
Member Author

This would be a change in how fk are added and would require an update to https://github.com/cockroachdb/cockroach/blob/master/docs/RFCS/20160426_fk.md as well.

@vivekmenezes
Copy link
Contributor

Can you add the exact statements are failing?

@BramGruneir
Copy link
Member Author 

create table CUSTOMER_TABLE (
   ID varchar(255) not null,
    AGE int4,
    code varchar(255),
    country varchar(255),
    NAME varchar(255),
    FK6_FOR_CUSTOMER_TABLE varchar(255),
    FK5_FOR_CUSTOMER_TABLE varchar(255),
    primary key (ID)
)
create table ALIAS_TABLE (
   ID varchar(255) not null,
    ALIAS varchar(255),
    FK1_FOR_CUSTOMER_TABLE varchar(255),
    primary key (ID)
)
alter table if exists ALIAS_TABLE 
       add constraint FKskt3r5srri8i12nq13uw2ihqi 
       foreign key (FK1_FOR_CUSTOMER_TABLE) 
       references CUSTOMER_TABLE
23:21:21,271  WARN ExceptionHandlerLoggedImpl:27 - GenerationTarget encountered exception accepting command : Error executing DDL "
    alter table if exists ALIAS_TABLE 
       add constraint FKskt3r5srri8i12nq13uw2ihqi 
       foreign key (FK1_FOR_CUSTOMER_TABLE) 
       references CUSTOMER_TABLE" via JDBC Statement
org.hibernate.tool.schema.spi.CommandAcceptanceException: Error executing DDL "
    alter table if exists ALIAS_TABLE 
       add constraint FKskt3r5srri8i12nq13uw2ihqi 
       foreign key (FK1_FOR_CUSTOMER_TABLE) 
       references CUSTOMER_TABLE" via JDBC Statement
	at org.hibernate.tool.schema.internal.exec.GenerationTargetToDatabase.accept(GenerationTargetToDatabase.java:67)
	at org.hibernate.tool.schema.internal.SchemaCreatorImpl.applySqlString(SchemaCreatorImpl.java:440)
	at org.hibernate.tool.schema.internal.SchemaCreatorImpl.applySqlStrings(SchemaCreatorImpl.java:424)
	at org.hibernate.tool.schema.internal.SchemaCreatorImpl.createFromMetadata(SchemaCreatorImpl.java:375)
	at org.hibernate.tool.schema.internal.SchemaCreatorImpl.performCreation(SchemaCreatorImpl.java:166)
	at org.hibernate.tool.schema.internal.SchemaCreatorImpl.doCreation(SchemaCreatorImpl.java:135)
	at org.hibernate.tool.schema.internal.SchemaCreatorImpl.doCreation(SchemaCreatorImpl.java:121)
	at org.hibernate.tool.schema.spi.SchemaManagementToolCoordinator.performDatabaseAction(SchemaManagementToolCoordinator.java:155)
	at org.hibernate.tool.schema.spi.SchemaManagementToolCoordinator.process(SchemaManagementToolCoordinator.java:72)
	at org.hibernate.internal.SessionFactoryImpl.<init>(SessionFactoryImpl.java:310)
	at org.hibernate.boot.internal.SessionFactoryBuilderImpl.build(SessionFactoryBuilderImpl.java:467)
	at org.hibernate.jpa.boot.internal.EntityManagerFactoryBuilderImpl.build(EntityManagerFactoryBuilderImpl.java:939)
	at org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase.buildEntityManagerFactory(BaseEntityManagerFunctionalTestCase.java:77)
	at jdk.internal.reflect.GeneratedMethodAccessor1128.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:24)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:298)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:292)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.lang.Thread.run(Thread.java:844)
Caused by: org.postgresql.util.PSQLException: ERROR: foreign key requires an existing index on columns ("fk1_for_customer_table")
	at org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(QueryExecutorImpl.java:2433)
	at org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:2178)
	at org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:306)
	at org.postgresql.jdbc.PgStatement.executeInternal(PgStatement.java:441)
	at org.postgresql.jdbc.PgStatement.execute(PgStatement.java:365)
	at org.postgresql.jdbc.PgStatement.executeWithFlags(PgStatement.java:307)
	at org.postgresql.jdbc.PgStatement.executeCachedSql(PgStatement.java:293)
	at org.postgresql.jdbc.PgStatement.executeWithFlags(PgStatement.java:270)
	at org.postgresql.jdbc.PgStatement.execute(PgStatement.java:266)
	at org.hibernate.tool.schema.internal.exec.GenerationTargetToDatabase.accept(GenerationTargetToDatabase.java:54)
	... 24 more
23:21:21,271 DEBUG SQL:94 -

@BramGruneir
Copy link
Member Author

BramGruneir commented Jun 18, 2018 •
edited
Loading

Just to update, at least two tests in the suite fails because it specifically was testing FK constraints and they are not added in any hibernate test due to this.

  • org.hibernate.jpa.test.exception.ExceptionTest
    • testConstraintViolationException
  • org.hibernate.test.annotations.onetomany.OneToManyTest
    • testCascadeDeleteWithUnidirectionalAssociation

@knz
Copy link
Contributor

knz commented Jun 22, 2018

👍 on the idea to issue a select query on the table with LIMIT 1 upon every index/constraint creation and use a different path (perhaps more flexible and/or faster, since it doesn't need to backfill).

@BramGruneir BramGruneir mentioned this issue Jul 16, 2018
Closed
@knz knz added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Jul 21, 2018
@verglor
Copy link

verglor commented Oct 8, 2018

This is also blocker for Grails, since no FKs are created because of missing indexes.
Workaround of manually adding indexes all over the domain model is really ugly.
Is there any ETA or plan for this issue, please ?

@knz
Copy link
Contributor

knz commented Oct 8, 2018

There is a plan to investigate this and determine what parts of CockroachDB can be changed in the next 3-4 months. So the ETA is not before the CockroachDB 2.2 release, some time after Q2 2019.

@knz knz mentioned this issue Oct 23, 2018
Closed
@vivekmenezes
Copy link
Contributor

This is a good starter project

@vivekmenezes vivekmenezes added the E-easy Easy issue to tackle, requires little or no CockroachDB experience label Oct 24, 2018
@thoszhang thoszhang self-assigned this Oct 31, 2018
@thoszhang thoszhang mentioned this issue Nov 12, 2018
Merged
thoszhang pushed a commit to thoszhang/cockroach that referenced this issue Nov 13, 2018
@lucy-zhang
sql: automatically add index for fk constraint on empty table
fa3379f 
Adding a foreign key constraint requires that the referencing column(s)
be indexed, so previously, an ALTER TABLE statement to add a foreign key
constraint for an existing table would always fail, even if the
referencing table were empty. This change automatically adds the needed
index for the foreign key constraint when the table is empty. This will
improve compatibility with some ORMs, which create tables before adding
foreign key constraints.

Fixes cockroachdb#26738

Release note (sql change): An ALTER TABLE statement to add a foreign key
constraint now automatically creates the necessary index if the
referencing table is empty, if the index does not already exist.
thoszhang pushed a commit to thoszhang/cockroach that referenced this issue Nov 14, 2018
@lucy-zhang
sql: automatically add index for fk constraint on empty table
e3128c3 
Adding a foreign key constraint requires that the referencing column(s)
be indexed, so previously, an ALTER TABLE statement to add a foreign key
constraint for an existing table would always fail, even if the
referencing table were empty. This change automatically adds the needed
index for the foreign key constraint when the table is empty. This will
improve compatibility with some ORMs, which create tables before adding
foreign key constraints.

Fixes cockroachdb#26738

Release note (sql change): An ALTER TABLE statement to add a foreign key
constraint now automatically creates the necessary index if the
referencing table is empty, if the index does not already exist.
thoszhang pushed a commit to thoszhang/cockroach that referenced this issue Nov 20, 2018
@lucy-zhang
sql: automatically add index for fk constraint on empty table
d67ac45 
Adding a foreign key constraint requires that the referencing column(s)
be indexed, so previously, an ALTER TABLE statement to add a foreign key
constraint for an existing table would always fail, even if the
referencing table were empty. This change automatically adds the needed
index for the foreign key constraint when the table is empty. This will
improve compatibility with some ORMs, which create tables before adding
foreign key constraints.

Fixes cockroachdb#26738

Release note (sql change): An ALTER TABLE statement to add a foreign key
constraint now automatically creates the necessary index if the
referencing table is empty, if the index does not already exist.
craig bot pushed a commit that referenced this issue Nov 20, 2018
@lucy-zhang
Merge #32234
9be1de4 
32234: sql: automatically add index for fk constraint on empty table r=lucy-zhang a=lucy-zhang

Adding a foreign key constraint requires that the referencing column(s)
be indexed, so previously, an ALTER TABLE statement to add a foreign key
constraint for an existing table would always fail, even if the
referencing table were empty. This change automatically adds the needed
index for the foreign key constraint when the table is empty. This will
improve compatibility with some ORMs, which create tables before adding
foreign key constraints.

Fixes #26738

Release note (sql change): An ALTER TABLE statement to add a foreign key
constraint now automatically creates the necessary index if the
referencing table is empty, if the index does not already exist.

Co-authored-by: Lucy Zhang <lucy-zhang@users.noreply.github.com>
@craig craig bot closed this as completed in #32234 Nov 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@BramGruneir BramGruneir

@thoszhang thoszhang

Labels
A-schema-changes A-tools-hibernate Issues that pertain to Hibernate integration. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) E-easy Easy issue to tackle, requires little or no CockroachDB experience
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@knz @verglor @BramGruneir @thoszhang @vivekmenezes