SHOW CREATE TABLE returns invalid syntax for FK workaround

[petervandivier]

Describe the problem

Inline foreign key creation bails with the following error when a user attempts to create a redundant foreign key reference.

pq: column "a" cannot be used by multiple foreign key constraints

It is still possible to create a redundant (or partially redundant) reference using a workaround as noted here: #38850

However, examining the DDL state using SHOW CREATE TABLE after using such a workaround returns a command that cannot be executed to create the table.

To Reproduce

Consider the following valid postgresql schema

create table a ( 
	a int primary key
);

create table b (
	b int primary key,
	a int references a(a),
	unique(b,a)
);

create table c (
	c int primary key,
	a int references a(a),
	unique(c,a)
);

create table d (
	d int primary key,
	c int,
	b int,
	a int references a(a),
	constraint d_c foreign key (c,a) references c(c,a),
	constraint d_b foreign key (b,a) references b(b,a)
);

When creating table d, user is required to use a workaround due to the redundant foreign key references.

create table d (
	d int primary key,
	c int,
	b int,
	a int references a(a)
); 

alter  table d add constraint d_c foreign key (c,a) references c(c,a)

Foreign Key d_b is detected to be a partial duplicate of d_b and is rejected. However, when we inspect the current state of d, we see...

root@:26257/foo> show create table d;
  table_name |                      create_statement                        
+------------+-------------------------------------------------------------+
  d          | CREATE TABLE d (                                             
             |     d INT8 NOT NULL,                                         
             |     c INT8 NULL,                                             
             |     b INT8 NULL,                                             
             |     a INT8 NULL,                                             
             |     CONSTRAINT "primary" PRIMARY KEY (d ASC),                
             |     CONSTRAINT fk_a_ref_a FOREIGN KEY (a) REFERENCES a (a),  
             |     INDEX d_auto_index_fk_a_ref_a (a ASC),                   
             |     CONSTRAINT d_c FOREIGN KEY (c, a) REFERENCES c (c, a),   
             |     INDEX d_auto_index_d_c (c ASC, a ASC),                   
             |     FAMILY "primary" (d, c, b, a)                            
             | )                                                            
(1 row)

Time: 45.12ms

...which is not valid syntax for this version...

root@:26257/foo> CREATE TABLE d (
    d INT8 NOT NULL, 
    c INT8 NULL, 
    b INT8 NULL, 
    a INT8 NULL, 
    CONSTRAINT "primary" PRIMARY KEY (d ASC),
    CONSTRAINT fk_a_ref_a FOREIGN KEY (a) REFERENCES a (a),
    INDEX d_auto_index_fk_a_ref_a (a ASC),
    CONSTRAINT d_c FOREIGN KEY (c, a) REFERENCES c (c, a),
    INDEX d_auto_index_d_c (c ASC, a ASC),
    FAMILY "primary" (d, c, b, a) 
);
pq: column "a" cannot be used by multiple foreign key constraints

Expected behavior

Output from SHOW CREATE TABLE should be a valid executable syntax.

Additional data

Note: cockroach dump does return valid syntax in this scenario but at the expense of removing the in-lined constraint syntax.

Environment:

• CockroachDB version [e.g. 2.0.x]

root@:26257/foo> select version();
                                          version                                          
+-----------------------------------------------------------------------------------------+
  CockroachDB CCL v19.1.4 (x86_64-unknown-linux-gnu, built 2019/08/06 15:34:13, go1.11.6)  

• Server OS: [e.g. Linux/Distrib]

[centos@crdb-small-01 ~]$ uname -a
Linux crdb-small-01 3.10.0-957.1.3.el7.x86_64 #1 SMP Thu Nov 29 14:49:43 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

• Client app
  • cockroach sql
  • pg_dump
  • cockroach dump

Additional context

This was encountered when migrating an existing postgresql schema to cockroachdb.

Possibly related: #12718

[thoszhang]

Unfortunately, the problem is that

create table d (
	d int primary key,
	c int,
	b int,
	a int references a(a)
); 

alter  table d add constraint d_c foreign key (c,a) references c(c,a)

should never have worked, because we don't attempt to support multiple foreign key constraints on the same column, which is the deficiency being tracked in #38850.

I think the "workaround" mentioned in #38850 is actually the workaround of creating a duplicate of the column for each additional foreign key required, which is supported in Cockroach (by, for example, using computed columns), though it's inconvenient. @jordanlewis are you aware of any other workarounds for this?

The behavior being reported is due to a bug where we we failed to include foreign keys on the primary key columns when checking that no column has more than one foreign key constraint. This was patched in 19.2, and I wouldn't recommend relying on the erroneous behavior in 19.1, even if there are no apparent errors when creating the table.

We've run into this before (#37590 (comment)). Maybe we should backport a fix into 19.1.

[thoszhang]

I opened #42486 to track backporting a fix to 19.1 to close the loophole.

[petervandivier]

Fair enough; though I can’t help but wonder what the impact might be to installations that have already used this soon to be unsupported workaround. To be sure, redundant foreign key relationships raise questions about the quality of the data model, but... to revoke functionality on principle? Seems like an aggressive choice.

Fortunately for me, I’m still writing my conversions from Postgres to cockroach (so I’ve got time to refactor), but I have to assume there’s someone who’s got an existing redundant reference you’re proposing to disallow 😬

[thoszhang]

This is a good point. We're already planning to add support for multiple FK constraints on the same column soon (ideally in 20.1, though it's not guaranteed), so the only question is what to do in the meantime.

Based on what I've gathered from conversations with @knz, @jordanlewis, and others, there aren't any known problems in theory with multiple FK constraints from the same column. The restriction was put in place in a time when FKs were represented differently, and it wasn't clear that there was user demand, but in theory it should be able to be lifted now. The problem is that this functionality is untested and likely has unexpected behavior in edge cases, so any "partial support" for it via the loophole is potentially bad.

If a user had a table schema that bypassed this restriction, then it should probably continue to work in 19.2 just as well as it did in 19.1. (I realized there's an additional complication in this case, though, which is that the fix introduced in 19.2 to close the loophole inadvertently also prevents new foreign keys from being added if a user already has foreign keys that bypass the restriction. This is because the check indiscriminately applies to both new and old foreign keys. It's easy enough to get out of this state by just dropping one of the duplicate FKs, but it just adds to the overall state of bugginess.)

I think there are a few ways for us to proceed, until the restrictions are lifted in some later release:

1. Do nothing, so 19.1.x releases will continue to allow multiple FKs in the special primary key case, and 19.2.x releases will continue not to
2. Backport a fix to 19.2 to put the loophole back in, for consistency with 19.1
3. Backport a fix to 19.1 to close the loophole from now on (as proposed in sql: multiple FK constraints on the same column are erroneously allowed in 19.1 #42486)

There are other variants (e.g., we could backport a fix to 19.2 to close the loophole for new FKs while fixing the unintended consequences for exising FKs that violate the restriction, thus addressing the complication described above).

I think (3) would be best, in the interest of avoiding potential undefined behavior as much as possible. But I'll defer to @jordanlewis or someone else on the SQL team.

[solongordon]

After discussing in SQL Features planning, our plan is to:

• Do nothing for 19.2 right now.
• Continue making progress in 20.1 towards supporting multiple outgoing FKs on the same column.
• If easy, backport a 19.1 fix to prevent this situation.

[RoachietheSupportRoach]

Zendesk ticket #4291 has been linked to this issue.

[thoszhang]

We now allow multiple FK constraints on the same column in 19.2, and we've decided to not backport a fix to 19.1 (either to lift the restriction or to uniformly enforce it correctly).