| From: | "Alberto Piai" <alberto(dot)piai(at)gmail(dot)com> |
|---|---|
| To: | "jian he" <jian(dot)universality(at)gmail(dot)com>, "Alberto Piai" <alberto(dot)piai(at)gmail(dot)com> |
| Cc: | "PostgreSQL-development" <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: ALTER COLUMN SET EXPRESSION on partitions not work in case of constraint dependencies |
| Date: | 2026-08-24 18:57:28 |
| Message-ID: | DKXEJUNM4E94.MDWSQYIZBEZ1@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon Aug 10, 2026 at 10:28 AM CEST, jian he wrote:
> On Fri, Aug 7, 2026 at 9:35 PM Alberto Piai <alberto(dot)piai(at)gmail(dot)com> wrote:
>>
>> It's admittedly a long shot, since I'm still studying all of this, but:
>> what if, when we RememberConstraintForRebuilding() a constraint with
>> conislocal=false, we also RememberConstraintForRebuilding() its
>> corresponding parent constraint?
>>
>> I have not fully considered the implications of this and I'm only
>> thinking about CHECK and NN constraints.
>>
> That will make the entire hierarchy take an AccessExclusiveLock,
> which is not ideal and may require consensus.
> I think if the partition's generated column has an index on it, then
> directly changing its generation expression will not work.
You're right, please disregard my suggestion.
> Please check the two attached patches:
> v3-0001-Disallow-directly-ALTER-TABLE-SET-EXPRESSION-on-child-table-if-de.patch
> is for disallow directly running ALTER TABLE SET EXPRESSION on child table if
> dependencies (indexes, constraints) exist.
>
> v3-0001-Fix-dependency-issue-when-directly-ALTER-TABLE-SET-EXPRESSION-o.nocfbot
> It allows directly running ALTER TABLE SET EXPRESSION on a partition, if that
> partition only has constraint dependencies, not index dependencies. This is a
> POC with extensive tests.
> Rationale: changing the generation expression only affects the underlying column
> data, not its type. So instead of rebuilding these constraints, we can just
> re-validate them against the newly computed data. This avoids unnecessary
> constraint rebuilds and gets rid of the ACCESS EXCLUSIVE lock requirement on the
> whole partition hierarchy when we're only touching one partition.
Thanks for the new patches, I spent some time thinking about these
problems and the situation is clearer now.
You found several corner cases, and it's quite hard to deal with them at
once. Please allow me to focus on one case at first: constraints (let's
leave indexes aside for now). We might get to a patch that is easier to
reason about and maybe even backpatch.
I think your observation that we don't need to rebuild the constraints
but only verify them is the key here.
If you go this route, based on your second patch
(v3-0001-Fix-dependency-issue...):
- we wouldn't need to add the function ATPrepSetExpression (for now at
least)
- this also avoids the problem that with your current patch marking the
table with tab->rewrite |= AT_REWRITE_DEFAULT_VAL is done twice, once
in your new ATPrepSetExpression and then again in ATExecSetExpression
- rather than removing the constraints from the tab->changedConstraints*
lists, maybe it would be possible to change the Remember*ForRebuilding
functions to do the right thing in this case? They already know that
subtype=AT_SetExpression, after all
What do you think?
As a user, I would find it very surprising if ALTER TABLE ONLY ... SET
EXPRESSION failed sometimes, depending on how dependencies are set up.
It would be great if we'd manage to get this working.
Thanks for working on this!
Alberto
--
Alberto Piai
Sensational AG
Zürich, Switzerland
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | Pavel Stehule | 2026-08-24 18:42:34 | Re: missing possibility to use alternative translated month names in to_char function |