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. 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
