sql: Can't drop and replace a table within a transaction

[a-robinson]

I'm unable to drop a table and replace it with a different table of of the same name within a transaction. This is something that postgres allows.

In cockroach:

root@:26257> CREATE TABLE foo (k INT, v INT);
CREATE TABLE
root@:26257> BEGIN; DROP TABLE foo; CREATE TABLE foo (k INT, v INT); COMMIT;
pq: relation "foo" already exists

In postgres:

alex=# CREATE TABLE foo (k INT, v INT);
CREATE TABLE
alex=# BEGIN; DROP TABLE foo; CREATE TABLE foo (k INT, v INT); COMMIT;
BEGIN
DROP TABLE
CREATE TABLE
COMMIT

This particular example is a little contrived -- the example I was using when I ran into this was the series of commands suggested in #5887 where you drop a table and rename a slightly different table over it. I don't actually need to run the commands from #5887 in a transaction because they'll only be run as part of a stop-the-world upgrade, but users will have situations where they need to.

I think this may be a known issue judging by @andreimatei's comment on #7348, but I didn't see any issues tracking this limitation.

[vivekmenezes]

We can't support this without a major headache because we allow caching of the table descriptor and a deleted table is only truly deleted once all the nodes have given up their leases on the original table descriptor.

I think we could definitely improve the error message though to:
pq: relation "foo" in the process of being deleted

[cuongdo]

Is it possible to block the CREATE TABLE if there's a pending DROP, then unblock it when the DROP TABLE finishes? That would seem like the generally expected behavior for a SQL database.

…

On Wed, Dec 7, 2016 at 9:26 AM vivekmenezes ***@***.***> wrote: We can't support this without a major headache because we allow caching of the table descriptor and a deleted table is only truly deleted once all the nodes have given up their leases on the original table descriptor. I think we could definitely improve the error message though to: pq: relation "foo" in the process of being deleted — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#12123 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABffpsoXqnanXXpe09YnhJ6HI35IlYomks5rFsH7gaJpZM4LGL9I> .

[dt]

Isn't the issue here not that we async drop, but rather that we have the somewhat surprising behavior of keeping the name mapped to the same table ID for the life of the txn, even if the table is dropped or renamed?

e.g. this still seems very strange to me:

BEGIN;
ALTER TABLE bar RENAME TO foo;
SELECT * FROM bar;
+---+
| i |
+---+
+---+
COMMIT```

[andreimatei]

Schema changes in transactions are just weird.
The DROP is performed async, when the transaction commits.

Is it possible to block the CREATE TABLE if there's a pending DROP, then
unblock it when the DROP TABLE finishes?

No... At least not without reworking how descriptor leasing works. The transaction doing the DROP needs to release its own lease on the old version.

we have the somewhat surprising behavior of keeping the name mapped to the same table ID for the life of the txn, even if the table is dropped or renamed

The idea is that you can't have the same name refer to 2 things at the same time, in different transactions. So we keep the old name around until everybody has stopped using it.

[bdarnell]

The transaction doing the DROP needs to release its own lease on the old version.

But isn't that lease on the table ID, not the name? We could remove the name from the name-to-ID mapping immediately, leaving the transaction with a lease on an unlinked table ID.

The idea is that you can't have the same name refer to 2 things at the same time, in different transactions.

What's wrong with that? That's exactly what temporary tables are. Conceptually, the name-to-ID mapping should be equivalent to doing a consistent read from the namespace table within the transaction (so it would see the result of its own DROP but other transactions' DROPs and CREATES wouldn't take effect until they commit). I realize that we don't want to do this consistent read on every transaction for performance reasons, but I think that's the model we want to aim for.

[andreimatei]

I don't disagree, but currently the table name cache is tied to leased descriptors, and it's global per node. There's no per-transaction override. Even if we made it work for the node performing the DROP/CREATE, I'm not sure how we'd provide the illusion of the "consistent read from the namespace table" to other nodes, which rely on new descriptor versions being published through gossip to invalidate their caches. So, when a name is flipped to refer to a new table in a txn, it's hard to get all the other nodes to consistently resolve the name soon after the said txn has committed.

[vivekmenezes]

45614b6

fixes this issue by disallowing such transactions.

[vivekmenezes]

#14368 fixes this

[aeneasr]

The same appears to be true for BEGIN TRANSACTION; ALTER TABLE bar RENAME TO foo; CREATE TABLE bar; COMMIT;

[ajwerner]

This really is a special case of #54562.

Fixing this represents a trade off. Because of that way that cockroach's online schema change protocol works, if we allowed this, there would be some period of time where the name referred to both tables. Maybe that's acceptable.

[ajwerner]

This has now been fixed with #54562.