On Tue, Jan 8, 2019 at 3:02 PM Michael Paquier <mich...@paquier.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
