Split out of #248, which fixed the same class for per-table options.
The gap
pgcolumnar.add_projection() declares a projection; the row lives in
pgcolumnar.projection. pg_dump does not carry it, so a restored table has no
projections and silently loses whatever read performance they were providing.
Nothing errors: the data is intact, the queries still answer, they are just
slower and nobody is told why.
Why the #248 fix does not extend to it
#248 registered pgcolumnar.options with pg_extension_config_dump, which works
because that table is keyed by regclass -- a name that survives the dump and
restore OID remap.
pgcolumnar.projection is keyed by storage_id, and carries proj_storage_id
as well:
storage_id bigint -- the table's base storage id
projection_id integer
name name
proj_storage_id bigint -- this projection's own storage id
sort_key smallint[]
columns smallint[]
Storage ids are assigned when a relation is created, so a restore generates new
ones. Registering this table for config_dump would restore rows pointing at
storage that does not exist, which is worse than losing them: corruption instead
of absence.
What it needs instead
Re-emitting the intent rather than the rows -- the restored database needs
pgcolumnar.add_projection(rel, name, ...) calls, not the old catalog contents.
pg_dump has no mechanism for an extension to contribute arbitrary SQL for a
table it does not own, so the plausible shapes are:
- a documented
pgcolumnar.dump_projections() returning the DDL to re-run,
which puts the burden on the operator but is honest and cheap;
- storing projection definitions in a regclass-keyed table so config_dump can
carry them, with storage ids resolved at first use rather than at declaration;
- accepting the loss and documenting it in
docs/limitations.md.
Option 2 is the only one that makes a plain pg_dump | psql correct, and it is a
schema change. Worth deciding before 1.0 rather than after, since it changes the
catalog.
Test
test/pg_dump_roundtrip.sh already has the shape: pin the current behaviour as
an assertion so a fix turns it red, exactly as #249 did for options.
Split out of #248, which fixed the same class for per-table options.
The gap
pgcolumnar.add_projection()declares a projection; the row lives inpgcolumnar.projection.pg_dumpdoes not carry it, so a restored table has noprojections and silently loses whatever read performance they were providing.
Nothing errors: the data is intact, the queries still answer, they are just
slower and nobody is told why.
Why the #248 fix does not extend to it
#248 registered
pgcolumnar.optionswithpg_extension_config_dump, which worksbecause that table is keyed by regclass -- a name that survives the dump and
restore OID remap.
pgcolumnar.projectionis keyed by storage_id, and carriesproj_storage_idas well:
Storage ids are assigned when a relation is created, so a restore generates new
ones. Registering this table for config_dump would restore rows pointing at
storage that does not exist, which is worse than losing them: corruption instead
of absence.
What it needs instead
Re-emitting the intent rather than the rows -- the restored database needs
pgcolumnar.add_projection(rel, name, ...)calls, not the old catalog contents.pg_dumphas no mechanism for an extension to contribute arbitrary SQL for atable it does not own, so the plausible shapes are:
pgcolumnar.dump_projections()returning the DDL to re-run,which puts the burden on the operator but is honest and cheap;
carry them, with storage ids resolved at first use rather than at declaration;
docs/limitations.md.Option 2 is the only one that makes a plain
pg_dump | psqlcorrect, and it is aschema change. Worth deciding before 1.0 rather than after, since it changes the
catalog.
Test
test/pg_dump_roundtrip.shalready has the shape: pin the current behaviour asan assertion so a fix turns it red, exactly as #249 did for options.