| From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, pgsql-hackers(at)lists(dot)postgresql(dot)org, jian he <jian(dot)universality(at)gmail(dot)com> |
| Subject: | Re: Fix domain fast defaults on empty tables |
| Date: | 2026-06-08 15:40:27 |
| Message-ID: | 12802538-383a-4464-9a7d-de1d98bd6d08@dunslane.net |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 2026-06-06 Sa 10:09 PM, Chao Li wrote:
>
>> On Jun 7, 2026, at 04:19, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>
>> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>>> On 2026-06-05 Fr 10:08 AM, Tom Lane wrote:
>>>> Concretely, I'm pretty sure it is a hazard for pg_dump, which thinks
>>>> it can freely transform bits of CREATE operations into ALTERs.
>>>> I didn't try to make an example case, but I suspect it is now possible
>>>> to create a database that will fail dump/restore because of this
>>>> inconsistency.
>>> Seems reasonable. So which of Chao's solutions do you prefer? I think
>>> both will meet the pg_dump issue, not sure how much we care about the
>>> case where we have deleted all the rows but not truncated the table.
>> [ studies it a bit ...] TBH, I don't like either of these,
>> nor do I like a0b6ef29a to begin with. The fundamental problem
>> with this whole mess is that it treats certain kinds of
>> default-expression evaluation error (i.e., domain CHECK failures)
>> differently from others. There is no way that that leads to a
>> consistent user experience. The current complaint is one
>> manifestation of that, but I'm sure there are others, eg having
>> to do with domain coercions lower down in a default expression.
>>
>> It could work if ExecPrepareExprWithContext were capable of
>> soft-trapping essentially all execution errors, but that's
>> not true today and I strongly doubt it ever will be true.
>>
>> I think Chao's v1-s2-0001 points the way towards what could be a
>> workable solution: if we see that the table is known empty (after
>> we already have exclusive lock on it!), we could skip both the table
>> rewrite and the insertion of an attmissingval, and thereby not need
>> to evaluate the default at all during ALTER TABLE. As Chao says,
>> simplistic versions of "known empty" would expose some user-visible
>> behavioral inconsistencies, but I'm not sure how much that matters.
>> For pg_dump and similar applications we would get the behavior we
>> needed even with just an is-physically-empty check, since all their
>> CREATE/ALTER DDL happens before we ever insert any data. You could
>> imagine going further and scanning the table for live tuples, but
>> I don't know that that's going to be worth the cycles.
>>
>> However, I doubt that a bare "RelationGetNumberOfBlocks() == 0" check
>> is acceptable: we probably need to let the table access method have
>> control of this. heapam could do "RelationGetNumberOfBlocks() == 0",
>> but other TAMs might need to do something else. So that means that
>> this path requires a new TableAmRoutine method, and that probably
>> puts it in the too-late-for-v19 category.
>>
>> My recommendation: we ought to revert a0b6ef29a for now and
>> redesign the optimization for v20.
>>
>> If you don't like that answer, I'd be firmly against v1-s1-0001
>> in any case. It repeats the classic mistake of supposing that
>> we can PG_CATCH random errors and not invoke transaction cleanup.
>>
>> BTW, I do not like the fact that a0b6ef29a removed the comment
>> stanza explaining why we need this fake default expression at all.
>> That's still largely applicable AFAICS.
>>
>> regards, tom lane
> Thanks, Tom, for the detailed explanation.
>
> I'll hold off here and wait for Andrew's decision on how to proceed. Sounds like reverting a0b6ef29a may be the preferred option for now.
>
> For v1-s2, I’m not ware of a better way to decide whether a table is known empty. I ever considered a count(*)-style scan or a LIMIT 1 scan, but those seem worse, they would add a new table scan inside ALTER TABLE just to decide whether to evaluate the default.
>
Given Tom's reasonable suggestion that we really need a new Table AM
callback to do this sensibly, I agree we should revert it. Will do.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nazir Bilal Yavuz | 2026-06-08 16:10:46 | Re: ci: CCache churns through available space too quickly |
| Previous Message | Nathan Bossart | 2026-06-08 15:36:26 | Re: expand refint docs with usage info |