Re: Displaying and dumping of table access methods

On Tue, Jan 8, 2019 at 3:02 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:

> On Mon, Jan 07, 2019 at 06:31:52PM -0800, Andres Freund wrote:
> > Huh? It's absolutely *trivial* from a buildsystem POV to run the tests
> > again with a different default AM. That's precisely why I'm talking
> > about this. Just setting PGOPTIONS='-c
> > default_table_access_method=zheap' in the new makefile target (the ms
> > run scripts are similar) is sufficient. And we don't need to force
> > everyone to constantly run tests with e.g. both heap and zheap, it's
> > sufficient to do so on a few buildfarm machines, and whenever changing
> > AM level code. Rerunning all the tests with a different AM is just
> > setting the same environment variable, but running check-world as the
> > target.
>

PGOPTIONS or any similar options are good for the AM development
to test their AM's with all the existing PostgreSQL features.

> Another point is that having default_table_access_method facilitates
> the restore of tables across AMs similarly to tablespaces, so CREATE
> TABLE dumps should not include the AM part.
>

+1 to the above approach to dump "set default_table_access_method".

> > And even if you were to successfully argue that it's sufficient during
> > normal development to only have a few zheap specific additional tests,
> > we'd certainly want to make it possible to occasionally explicitly run
> > the rest of the tests under zheap to see whether additional stuff has
> > been broken - and that's much harder to sift through if there's a lot of
> > spurious test failures due to \d[+] outputting additional/differing
> > data.
>
> The specific-heap tests could be included as an extra module in
> src/test/modules easily, so removing from the main tests what is not
> completely transparent may make sense. Most users use make-check to
> test a patch quickly, so we could miss some bugs because of that
> during review. Still, people seem to be better-educated lately in the
> fact that they need to do an actual check-world when checking a patch
> at full. So personally I can live with a split where it makes sense.
> Being able to easily validate an AM implementation would be nice.
> Isolation tests may be another deal though for DMLs.
>
> > We are working seriously hard on making AMs pluggable. Zheap is not yet,
> > and won't be that soon, part of core. The concerns for an in-core zheap
> > (which needs to maintain the test infrastructure during the remainder of
> > its out-of-core development!) and out-of-core AMs are pretty aligned. I
> > don't get your confusion.
>
> I would imagine that a full-fledged AM should be able to maintain
> compatibility with the full set of queries that heap is able to
> support, so if you can make the tests transparent enough so as they
> can be run for any AMs without alternate input in the core tree, then
> that's a goal worth it. Don't you have plan inconsistencies as well
> with zheap?
>
> In short, improving portability incrementally is good for the
> long-term prospective. From that point of view adding the AM to \d+
> output may be a bad idea, as there are modules out of core which
> rely on psql meta-commands, and it would be nice to be able to test
> those tests as well for those plugins with different types of AMs.
>

I also agree that adding AM details to \d+ will lead to many unnecessary
failures. Currently \d+ also doesn't show all the details of the relation
like
owner and etc.

Regards,
Haribabu Kommi
Fujitsu Australia