Drop MD tables when MD support is disabled #771
Conversation
Y--
force-pushed
the
yl/md-tables-depend-fdw
branch
from
fcb25a1 to
e1aa072
Compare
Y--
force-pushed
the
yl/md-tables-depend-fdw
branch
from
e1aa072 to
4f1078a
Compare
src/pgduckdb_background_worker.cpp
Outdated
| .objectSubId = 0, | ||
| }; | ||
| recordDependencyOn(&schema_address, &extension_address, DEPENDENCY_NORMAL); | ||
| recordDependencyOn(&schema_address, &server_address, DEPENDENCY_NORMAL); |
There was a problem hiding this comment.
This works for tables in a ddb$ schema, because those will then depend on the SERVER indirectly through their schema. But this doesn't yet work for tables in the default database, for those we need to create a direct dependency between the table and the schema. (and probably we just want to create the direct dependency for any duckdb table, no matter what schema they are in, just for consistency).
Y--
force-pushed
the
yl/md-tables-depend-fdw
branch
from
9655d2c to
9cbf120
Compare
| .objectId = oid, | ||
| .objectSubId = 0, | ||
| }; | ||
| recordDependencyOnMDServer(&object_address); |
There was a problem hiding this comment.
This should be done in our DDL hook instead. That way the dependency is also recorded when someone creates a motherduck table from Postgres, not just when a table is synced to by the background worker.
Y--
force-pushed
the
yl/md-tables-depend-fdw
branch
2 times, most recently
from
d39af6a to
6065296
Compare
Y--
force-pushed
the
yl/md-tables-depend-fdw
branch
2 times, most recently
from
d313255 to
42c3336
Compare
Y--
force-pushed
the
yl/md-tables-depend-fdw
branch
from
42c3336 to
7a99398
Compare
| } else { | ||
| // Something went really wrong (since `create_table_succeeded`) | ||
| elog(WARNING, "Failed to find table '%s.%s' so won't record dependency to 'motherduck' SERVER", table_name, | ||
| postgres_schema_name); |
There was a problem hiding this comment.
| postgres_schema_name); | |
| postgres_schema_name); | |
| RollbackAndReleaseCurrentSubTransaction(); | |
| return false; |
And as a nit: then we can change this to "early return" style if we change
if (HeapTupleIsValid(new_table_tuple)) {
to the inverse
if (!HeapTupleIsValid(new_table_tuple)) {
Currently PG catalog entries mirroring MD tables are dropped only when the extension is.
This PR changes this behavior to have them depend on the
motherduckSERVER so that when it is removed, the entries are removed as well.