Re: Displaying and dumping of table access methods

Greetings,

* Andres Freund (andres(at)anarazel(dot)de) wrote:
> Over in [1] we're discussing the development of the pluggable storage
> patchset, which allows different ways of storing table data.
>
> One thing I'd like to discuss with a wider audience than the
> implementation details is psql and pg_dump handling of table access
> methods.
>
> Currently the patchset neither dumps nor displays table access
> methods . That's clearly not right.

Agreed.

> The reason for that however is not that it's hard to dump/display
> code-wise, but that to me the correct behaviour is not obvious.

While it might be a lot of changes to the regression output results, I
tend to feel that showng the access method is a very important aspect of
the relation and therefore should be in \d output.

> The reason to make table storage pluggable is after all that the table
> access method can be changed, and part of developing new table access
> methods is being able to run the regression tests.

We certainly want people to be able to run the regression tests, but it
feels like we will need more regression tests in the future as we wish
to cover both the built-in heap AM and the new zheap AM, so we should
really have those both run independently. I don't think we'll be able
to have just one set of regression tests that cover everything
interesting for both, sadly. Perhaps there's a way to have a set of
regression tests which are run for both, and another set that's run for
each, but building all of that logic is a fair bit of work and I'm not
sure how much it's really saving us.

> A patch at [2] adds display of a table's access method to \d+ - but that
> means that running the tests with a different default table access
> method (e.g. using PGOPTIONS='-c default_table_access_method=...)
> there'll be a significant number of test failures, even though the test
> results did not meaningfully differ.

Yeah, I'm not really thrilled with this approach.

> Similarly, if pg_dump starts to dump table access methods either
> unconditionally, or for all non-heap AMS, the pg_dump tests fail due to
> unimportant differences.

In reality, pg_dump already depends on quite a few defaults to be in
place, so I don't see a particular issue with adding this into that set.
New tests would need to have new pg_dump checks, of course, but that's
generally the case as well.

> A third issue, less important in my opinion, is that specifying the
> table access method means that it's harder to dump/restore into a table
> with a different AM.

I understand this concern but I view it as an independent consideration.
There are a lot of transformations which one might wish for when dumping
and restoring data, a number of which are handled through various
options (--no-owner, --no-acls, etc) and it seems like we could do
something similar here.

> One way to solve this would be for psql/pg_dump to only define the table
> access methods for tables that differ from the currently configured
> default_table_access_method. That'd mean that only a few tests that
> intentionally test AMs would display/dump the access method. On the
> other hand that seems like it's a bit too much magic.

I'm not a fan of depending on the currently set
default_table_access_method.. When would that be set in the pg_restore
process? Or in the SQL script that's created? Really though, that does
strike me as quite a bit of magic.

> While I don't like that option, I haven't really come up with something
> better. Having alternative outputs for nearly every test file for
> e.g. zheap if/when we merge it, seems unmaintainable. It's less insane
> for the pg_dump tests.

I'm thinking less of alternative output files and more of additional
tests for zheap cases...

> An alternative approach based on that would be to hack pg_regress to
> magically ignore "Access method: ..." type differences, but that seems
> like a bad idea to me.

I agree, that's not a good idea.

Thanks!

Stephen