Michael Paquier <michael.paquier@gmail.com> writes:
> On Thu, Mar 6, 2014 at 8:46 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 2. We could remove the error complaint in CheckValidRowMarkRel(),
>> allowing a matview row to be marked the same as a regular-table row.
>>
>> Since matview rows do in fact have TIDs and the same
>> visibility/vacuumability rules as regular-table rows (no?), I see no
>> reason that #2 wouldn't work, though I admit I've not actually tried it.
> After digging into #2, I finished with the attached patch that passes
> regression tests. The idea is simply to allow a matview to use
> ROW_MARK_REFERENCE and ROW_MARK_COPY when its rows are referenced. I
> added a regression test as well in the patch.
The ROW_MARK_COPY test is unnecessary since control will never reach this
function for that case. (If it did, most of the other case branches would
need to be checking it; checking only here is inconsistent.)
Also, after comparing this to the code in CheckValidResultRel, I wonder
why we aren't allowing SELECT FOR UPDATE in maintenance mode; that is,
shouldn't the test go like this:
if (markType != ROW_MARK_REFERENCE &&
!MatViewIncrementalMaintenanceIsEnabled())
However, that smells less like a bug fix and more like a feature addition,
so I'll leave it to Kevin to decide whether that's appropriate. In the
meantime, I committed the clear bug fix with some cosmetic adjustments.
regards, tom lane