Hello, Let me apologize for continuing the discussion even though the deadline is approaching.
At Fri, 11 Jul 2014 09:49:55 +0900, Michael Paquier <michael.paqu...@gmail.com> wrote in <cab7npqtjezoz-fotibsejyg0eabngx2plqywodchyfxzfqy...@mail.gmail.com> > Updated patches attached. > > On Thu, Jul 10, 2014 at 7:13 PM, Kyotaro HORIGUCHI > <horiguchi.kyot...@lab.ntt.co.jp> wrote: > > > > The usage of this is a bit irregular as a (extension) module but > > it is the nature of this 'contrib'. The rearranged page type > > detection logic seems neater and keeps to work as intended. This > > logic will be changed after the new page type detection scheme > > becomes ready by the another patch. > > No disk format changes will be allowed just to make page detection > easier (Tom mentioned that earlier in this thread btw). We'll have to > live with what current code offers, > especially considering that adding > new bytes for page detection for gin pages would double the size of > its special area after applying MAXALIGN to it. That's awkward, but I agree with it. By the way, I found PageHeaderData.pd_flags to have 9 bits room. It seems to be usable if no other usage is in sight right now, if the formal method to identify page types is worth the 3-4 bits there. # This is a separate discussion from this patch itself. > > - "make check" runs "regress --use-existing" but IMHO the make > > target which is expected to do that is installcheck. I had > > fooled to convince that it should run the postgres which is > > built dedicatedly:( > > Yes, the patch is abusing of that. --use-existing is specified in this > case because the test itself is controlling Postgres servers to build > and fetch the buffer captures. This allows more flexible machinery > IMO. Although I doubt necessity of the flexibility seeing the current testing framework, I don't have so strong objection about that. Nevertheless, perhaps you are appreciated to put a notice on.. README or somewhere. > > - postgres processes are left running after > > test_default(custom).sh has stopped halfway. This can be fixed > > with the attached patch, but, to be honest, this seems too > > much. I'll follow your decision whether or not to do this. > > (bufcapt_test_sh_20140710.patch) > > I had considered that first, thinking that it was the user > responsibility if things are screwed up with his custom scripts. I > guess that the way you are doing it is a safeguard simple enough > though, so included with some editing, particularly reporting to the > user the error code returned by the test script. Agreed. > > - test_default.sh is not only an example script which will run > > while utilize this facility, but the test script for this > > facility itself. > > So I think it would be better be added some queries so that all > > possible page types available for the default testing. What do > > you think about the attached patch? (hash index is unlogged > > but I dared to put it for clarity.) > > Yeah, having a default set of queries run just to let the user get an > idea of how it works improves things. > Once again thanks for taking the time to look at that. Thank you. regardes, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
