On Fri, Aug 24, 2018 at 05:30:01PM +0000, Bossart, Nathan wrote:
> On 8/23/18, 9:16 PM, "Michael Paquier" <michael@paquier.xyz> wrote:
>> Thanks, I have pushed the new test series, and reused it to check the
>> new version of the main patch, which is attached. I have added a commit
>> message and I have indented the thing.
>
> Thanks for the new version!
Finally, I have been able to come back to it, and pushed the latest
version. We have come a long way... I'll check the rest of the backend
code for weird calls of relation_open or such. We may have other cases
with similar problems.
>> After pondering about it, I have also reworked the portion for
>> partitioned tables so as the list of partitions processed is unchanged
>> on HEAD, and we keep a consistent behavior compared to past versions.
>> If VACUUM processing for partitioned tables was something new in 11, I
>> think that we could have considered it, but changing silently something
>> that people may rely on for more than one year now is not very
>> appealing.
>
> Agreed. Even though we're not fixing the issue for partitions yet,
> this patch should still fix the originally reported authentication
> issue (which I see is highlighted in your commit message). I think
> there's still a slight behavior change with the ordering of the
> "skipped" log messages in some cases, but that doesn't seem terribly
> important. We might be able to work around this by storing all the
> information we need for the log message in the VacuumRelation and
> waiting to emit it until vacuum_rel() or analyze_rel(), but I doubt
> it's worth the complexity.
This one is definitely not worth worrying in my opinion, we still
process the same relations, and the order is preserved when using a
single relation.
> The new version of the patch applies cleanly, builds cleanly, and
> 'make check-world' succeeds. Also, I'm no longer able to reproduce
> the authentication issue involving 'VACUUM FULL' run by non-
> superusers, so it looks good to me.
Thanks for the help!
--
Michael
