Review shadow_v6 patch Hello
I did a recheck a newest version of this patch: 1. There is a wide agreement on implemented feature - nothing changed from previous review - it is not necessary comment it again. 2. v6 patch: patching cleanly, compilation without errors and warnings, all regress tests passed Tom's objections was related to GUC part. It is redesigned as Tom proposed. The code is good - and I don't see any problem there. I have only one objection - What I remember - more usual is using a list instead a bitmap for these purposes - typical is DefElem struct. Isn't it better? Regards Pavel 2014-03-20 12:39 GMT+01:00 Petr Jelinek <p...@2ndquadrant.com>: > On 20/03/14 00:32, Tom Lane wrote: > >> >> TBH, if I thought this specific warning was the only one that would ever >> be there, I'd probably be arguing to reject this patch altogether. >> > > Of course, nobody assumes that it will be the only one. > > > >> Also, adding GUC_LIST_INPUT later is not really cool since it changes >> the parsing behavior for the GUC. If it's going to be a list, it should >> be one from day zero. >> >> > Actually it does not since it all has to be handled in check/assign hook > anyway. > > But nevertheless, I made V6 with doc change suggested by Alvaro and also > added this list handling framework for the GUC params. > In the end it is probably less confusing now that the implementation uses > bitmask instead of bool when the user facing functionality talks about > list... > > This obviously needs code review again (I haven't changed tests since > nothing changed from user perspective). > > > > -- > Petr Jelinek http://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Training & Services >
