Re: BUG #6041: Unlogged table was created bad in slave node

On Tue, Jun 7, 2011 at 5:05 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Tue, Jun 7, 2011 at 3:53 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> It might be that it'd be best just to have both the planner and executor
>>> throwing errors on unlogged tables, rather than rejiggering pieces of
>>> the planner to sort-of not fail on an unlogged table.
>
>> Mmm, that's not a bad thought either.  Although I think if we can be
>> certain that the planner will error out, the executor checks aren't
>> necessary.  It would disallow preparing a statement and then executing
>> it after promotion, but that doesn't seem terribly important.  Any
>> idea where to put the check?
>
> Well, I'd recommend keeping the test in ExecOpenScanRelation, since it's
> cheap insurance against the situation changing since the plan was made.

Well, the system can't very well go back into recovery once it's
emerged. I guess it's possible that a table could be switched from
LOGGED to UNLOGGED during recovery though, in some hypothetical future
release. No one's proposed that feature yet, but since there IS a
proposal to go the other way it doesn't seem unlikely we may see the
other direction eventually too. I can't get too excited about
blocking this in multiple places just for the sake of preserving a
nicer error message in the face of a possible future feature
development, though.

> But for the planner, why not just put the same kind of test in
> get_relation_info, just after it does heap_open?

OK, let me try that.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company