On Wed, Feb 10, 2016 at 10:35 AM, Michael Paquier <michael.paqu...@gmail.com> wrote: > On Wed, Feb 10, 2016 at 2:23 AM, Fujii Masao <masao.fu...@gmail.com> wrote: >> On Wed, Feb 10, 2016 at 2:21 AM, Fujii Masao <masao.fu...@gmail.com> wrote: >>> On Tue, Feb 9, 2016 at 9:11 PM, Michael Paquier >>> <michael.paqu...@gmail.com> wrote: >>>> On Tue, Feb 9, 2016 at 4:27 PM, Fujii Masao <masao.fu...@gmail.com> wrote: >>>>> Thanks for updating the patch! >>>>> Attached is the updated version of the patch. >>>>> I removed unnecessary assertion check and change of source code >>>>> that you added, and improved the source comment. >>>>> Barring objection, I'll commit this patch. >>>> >>>> So, this code basically duplicates what is already in >>>> refresh_by_match_merge to check if there is a UNIQUE index defined. If >>>> we are sure that an error is detected earlier in the code as done in >>>> this patch, wouldn't it be better to replace the error message in >>>> refresh_by_match_merge() by an assertion? >>> >>> I'm OK with an assertion if we add source comment about why >>> refresh_by_match_merge() can always guarantee that there is >>> a unique index on the matview. Probably it's because the matview >>> is locked with exclusive lock at the start of ExecRefreshMatView(), >>> i.e., it's guaranteed that we cannot drop any indexes on the matview >>> after the first check is passed. Also it might be better to add >>> another comment about that the caller of refresh_by_match_merge() >>> must always check that there is a unique index on the matview before >>> calling that function, just in the case where it's called elsewhere >>> in the future. >>> >>> OTOH, I don't think it's not so bad idea to just emit an error, instead. >> >> Sorry, s/it's not/it's > > Well, refresh_by_match_merge is called only by ExecRefreshMatView() > and it is not exposed externally in matview.h, so it is not like we > are going to break any extension-related code by having an assertion > instead of an error message, and that's less code duplication to > maintain if this extra error message is removed. But feel free to > withdraw my comment if you think that's not worth it, I won't deadly > insist on that either :)
Okay, I revived Sawada's original assertion code and pushed the patch. Thanks! Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
