Re: Displaying and dumping of table access methods - Mailing list pgsql-hackers

Hi,

On 2019-01-07 20:30:13 -0500, Stephen Frost wrote:
> * Andres Freund (andres@anarazel.de) wrote:
> > On 2019-01-07 19:19:46 -0500, Stephen Frost wrote:
> > > * Andres Freund (andres@anarazel.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.
> >
> > I don't see how that'd be feasible.  Imo it is/was absolutely crucial
> > for zheap development to be able to use the existing postgres tests.
>
> I don't agree with the general assumption that "we did this for
> development and therefore it should be done that way forever".
>
> Instead, I would look at adding tests where there's a difference
> between the two, or possibly some difference, and make sure that there
> isn't, and make sure that the code paths are covered.

I think this approach makes no sense whatsover.  It's entirely possible
to encounter bugs in table AM relevant code in places one would not
think so. But even if one were, foolishly, to exclude those, the pieces
of code we know are highly affected by the way the AM works are a a
significant (at the very least 10-20% of tests) percentage of our
tests. Duplicating them even just between heap and zheap would be a
major technical debt.  DML, ON CONFLICT, just about all isolationtester
test, etc all are absolutely crucial to test different AMs for
correctness.


> > To me the approach you're suggesting is going to lead to an explosion of
> > redundant tests, which are really hard to maintain, especially for
> > out-of-tree AMs. Out of tree AMs with the setup I propose can just
> > install the AM into the template database and set PGOPTIONS, and they
> > have pretty good coverage.
>
> I'm frankly much less interested in out-of-tree AMs as we aren't going
> to have in-core regression tests for them anyway, because we can't as
> they aren't in our tree and, ultimately, I don't find them to have
> anywhere near the same value that in-core AMs have.

I think you must be missing my point: Adding spurious differences due to
"Table Access Method: heap" vs "Table Access Method: blarg" makes it
unnecessarily hard to reuse the in-core tests for any additional AM, be
it in-core or out of core.  I fail to see what us not having explicit
tests for such AMs in core has to do with my point.

Even just having a psql variable that says HIDE_NONDEFAULT_TABLE_AMS or
HIDE_TABLE_AMS that's set by default by pg_regress would be *vastly*
better from a maintainability POV than including the AM in the output.

Andres