public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v2] pg_dump: fix dependencies on FKs to multi-level partitioned tables
3+ messages / 3 participants
[nested] [flat]
* [PATCH v2] pg_dump: fix dependencies on FKs to multi-level partitioned tables
@ 2020-08-14 17:26 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Alvaro Herrera @ 2020-08-14 17:26 UTC (permalink / raw)
Parallel-restoring a foreign key that references a partitioned table
with several levels of partitions can fail:
pg_restore: while PROCESSING TOC:
pg_restore: from TOC entry 6684; 2606 29166 FK CONSTRAINT fk fk_a_fkey postgres
pg_restore: error: could not execute query: ERROR: there is no unique constraint matching given keys for referenced table "pk"
Command was: ALTER TABLE fkpart3.fk
ADD CONSTRAINT fk_a_fkey FOREIGN KEY (a) REFERENCES fkpart3.pk(a);
This happens in parallel restore mode because some index partitions
aren't yet attached to the topmost partitioned index that the FK uses,
and so the index is still invalid. The current code marks the FK as
dependent on the first level of index-attach dump objects; the bug is
fixed by recursively marking the FK on their children.
Reported-by: Tom Lane <[email protected]>
Author: �lvaro Herrera <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/bin/pg_dump/pg_dump.c | 39 ++++++++++++++++++++++++++++++++-------
1 file changed, 32 insertions(+), 7 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 9c8436dde6..2cb3f9b083 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -235,6 +235,7 @@ static DumpableObject *createBoundaryObjects(void);
static void addBoundaryDependencies(DumpableObject **dobjs, int numObjs,
DumpableObject *boundaryObjs);
+static void addConstrChildIdxDeps(DumpableObject *dobj, IndxInfo *refidx);
static void getDomainConstraints(Archive *fout, TypeInfo *tyinfo);
static void getTableData(DumpOptions *dopt, TableInfo *tblinfo, int numTables, char relkind);
static void makeTableDataInfo(DumpOptions *dopt, TableInfo *tbinfo);
@@ -7517,25 +7518,20 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables)
reftable = findTableByOid(constrinfo[j].confrelid);
if (reftable && reftable->relkind == RELKIND_PARTITIONED_TABLE)
{
- IndxInfo *refidx;
Oid indexOid = atooid(PQgetvalue(res, j, i_conindid));
if (indexOid != InvalidOid)
{
for (int k = 0; k < reftable->numIndexes; k++)
{
- SimplePtrListCell *cell;
+ IndxInfo *refidx;
/* not our index? */
if (reftable->indexes[k].dobj.catId.oid != indexOid)
continue;
refidx = &reftable->indexes[k];
- for (cell = refidx->partattaches.head; cell;
- cell = cell->next)
- addObjectDependency(&constrinfo[j].dobj,
- ((DumpableObject *)
- cell->ptr)->dumpId);
+ addConstrChildIdxDeps(&constrinfo[j].dobj, refidx);
break;
}
}
@@ -7548,6 +7544,35 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables)
destroyPQExpBuffer(query);
}
+/*
+ * addConstrChildIdxDeps
+ *
+ * Recursive subroutine for getConstraints
+ *
+ * Given an object representing a foreign key constraint and an index on the
+ * partitioned table it references, mark the constraint object as dependent
+ * on the DO_INDEX_ATTACH object of each index partition, recursively
+ * drilling down to their partitions if any. This ensures that the FK is not
+ * restored until the index is fully marked valid.
+ */
+static void
+addConstrChildIdxDeps(DumpableObject *dobj, IndxInfo *refidx)
+{
+ SimplePtrListCell *cell;
+
+ Assert(dobj->objType == DO_FK_CONSTRAINT);
+
+ for (cell = refidx->partattaches.head; cell; cell = cell->next)
+ {
+ IndexAttachInfo *attach = (IndexAttachInfo *) cell->ptr;
+
+ addObjectDependency(dobj, attach->dobj.dumpId);
+
+ if (attach->partitionIdx->partattaches.head != NULL)
+ addConstrChildIdxDeps(dobj, attach->partitionIdx);
+ }
+}
+
/*
* getDomainConstraints
*
--
2.20.1
--azLHFNyN32YCQGCU--
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: NOT ENFORCED constraint feature
@ 2025-02-12 14:45 Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Peter Eisentraut @ 2025-02-12 14:45 UTC (permalink / raw)
To: Álvaro Herrera <[email protected]>; Ashutosh Bapat <[email protected]>; +Cc: Isaac Morland <[email protected]>; Amul Sul <[email protected]>; jian he <[email protected]>; pgsql-hackers; Joel Jacobson <[email protected]>; Suraj Kharage <[email protected]>
On 12.02.25 12:13, Álvaro Herrera wrote:
> On 2025-Feb-12, Ashutosh Bapat wrote:
>
>> I have been asking a different question: What's the use of
>> not-enforced constraints if we don't allow VALID, NOT ENFORCED state
>> for them?
>
> That's a question for the SQL standards committee. They may serve
> schema documentation purposes, for example.
> https://www.postgresql.eu/events/pgconfeu2024/schedule/session/5677-exploring-postgres-databases-wit...
>
>> OTOH, consider an application which "knows" that the constraint is
>> valid for the data (either because of checks at application level, or
>> because the data was replicated from some other system where the
>> cosntraints were applied). It's a natural ask to use the constraints
>> for, say optimization, but don't take unnecessary overhead of
>> validating them. VALID, NOT ENFORCED state helps in such a scenario.
>> Of course an application can misuse it (just like stable marking on a
>> function), but well ... they will be penalised for their misuse.
>
> I disagree that we should see a VALID NOT ENFORCED constraint as one
> that can be used for query optimization purposes. This is only going to
> bring users pain, because it's far too easy to misuse and they will get
> wrong query results, possibly without knowing for who knows how long.
I've been digging into the ISO archives for some more background on the
intended meaning of this feature.
Result: "NOT ENFORCED" just means "off" or "disabled", "could contain
anything". You can use this to do data loads, or schema surgery, or
things like that. Or just if you want it for documentation.
This idea that a not-enforced constraint should contain valid data
anyway is not supported by anything I could find written down. I've
heard that in discussions, but those could have been speculations.
(I still think that could be a feature, but it's clearly not this one,
at least not in its default state.)
So considering that, I think a three-state system makes more sense.
Something like:
1) NOT ENFORCED -- no data is checked
2) NOT VALID -- existing data is unchecked, new data is checked
3) ENFORCED -- all data is checked
Transitions:
(1) - [ ALTER TABLE ... ALTER CONSTRAINT ... NOT VALID ] -> (2)
(1) - [ ALTER TABLE ... ALTER CONSTRAINT ... ENFORCED ] -> (3)
(2) - [ ALTER TABLE ... VALIDATE CONSTRAINT ... ] -> (3)
(2|3) - [ ALTER TABLE ... ALTER CONSTRAINT ... NOT ENFORCED ] -> (1)
(3) - [ ALTER TABLE ... ALTER CONSTRAINT ... NOT VALID ] -> (2)
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: NOT ENFORCED constraint feature
@ 2025-02-13 05:34 Ashutosh Bapat <[email protected]>
parent: Peter Eisentraut <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Ashutosh Bapat @ 2025-02-13 05:34 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Álvaro Herrera <[email protected]>; Isaac Morland <[email protected]>; Amul Sul <[email protected]>; jian he <[email protected]>; pgsql-hackers; Joel Jacobson <[email protected]>; Suraj Kharage <[email protected]>
On Wed, Feb 12, 2025 at 8:15 PM Peter Eisentraut <[email protected]> wrote:
>
> On 12.02.25 12:13, Álvaro Herrera wrote:
> > On 2025-Feb-12, Ashutosh Bapat wrote:
> >
> >> I have been asking a different question: What's the use of
> >> not-enforced constraints if we don't allow VALID, NOT ENFORCED state
> >> for them?
> >
> > That's a question for the SQL standards committee. They may serve
> > schema documentation purposes, for example.
> > https://www.postgresql.eu/events/pgconfeu2024/schedule/session/5677-exploring-postgres-databases-wit...
> >
> >> OTOH, consider an application which "knows" that the constraint is
> >> valid for the data (either because of checks at application level, or
> >> because the data was replicated from some other system where the
> >> cosntraints were applied). It's a natural ask to use the constraints
> >> for, say optimization, but don't take unnecessary overhead of
> >> validating them. VALID, NOT ENFORCED state helps in such a scenario.
> >> Of course an application can misuse it (just like stable marking on a
> >> function), but well ... they will be penalised for their misuse.
> >
> > I disagree that we should see a VALID NOT ENFORCED constraint as one
> > that can be used for query optimization purposes. This is only going to
> > bring users pain, because it's far too easy to misuse and they will get
> > wrong query results, possibly without knowing for who knows how long.
>
> I've been digging into the ISO archives for some more background on the
> intended meaning of this feature.
>
> Result: "NOT ENFORCED" just means "off" or "disabled", "could contain
> anything". You can use this to do data loads, or schema surgery, or
> things like that. Or just if you want it for documentation.
Hmm, so one can convert an enforced constraint to a not-enforced
constraint, load the data or make changes and then enforce it again.
Makes sense.
>
> This idea that a not-enforced constraint should contain valid data
> anyway is not supported by anything I could find written down. I've
> heard that in discussions, but those could have been speculations.
>
> (I still think that could be a feature, but it's clearly not this one,
> at least not in its default state.)
Thanks for the background.
>
> So considering that, I think a three-state system makes more sense.
> Something like:
>
> 1) NOT ENFORCED -- no data is checked
> 2) NOT VALID -- existing data is unchecked, new data is checked
> 3) ENFORCED -- all data is checked
>
> Transitions:
>
> (1) - [ ALTER TABLE ... ALTER CONSTRAINT ... NOT VALID ] -> (2)
Per your notation, this means the the constraint is not enforced but
new data is checked - that seems a contradiction, how would we check
the data when the constraint is not being enforced. Or do you suggest
that we convert a NOT ENFORCED constraint to ENFORCED as a result of
converting it to NOT VALID?
> (1) - [ ALTER TABLE ... ALTER CONSTRAINT ... ENFORCED ] -> (3)
Seems ok.
> (2) - [ ALTER TABLE ... VALIDATE CONSTRAINT ... ] -> (3)
As a result of this a not enforced constraint would turn into an
enforced constraint. The user might have intended to just validate the
data but not enforce it to avoid paying price for the checks on new
data.
> (2|3) - [ ALTER TABLE ... ALTER CONSTRAINT ... NOT ENFORCED ] -> (1)
Looks fine.
> (3) - [ ALTER TABLE ... ALTER CONSTRAINT ... NOT VALID ] -> (2)
>
This too seems ok assuming the constraint would remain enforced.
I think, what you intend to say is clearer with 4 state system {NE, E}
* {NV, V} = {(NE, NV), (NE, V), (E, NV), (E, V)} where (NE, V) is
unreachable. Let's name them S1, S2, S3, S4 respectively.
S1 -> [ ALTER TABLE ... ALTER CONSTRAINT ... NOT VALID ] -> S1 - noop
S3 -> [ ALTER TABLE ... ALTER CONSTRAINT ... NOT VALID ] -> S3 - noop
S4 -> [ ALTER TABLE ... ALTER CONSTRAINT ... NOT VALID ] -> S3
S1->[ ALTER TABLE ... ALTER CONSTRAINT ... ENFORCED ]->S4
S3->[ ALTER TABLE ... ALTER CONSTRAINT ... ENFORCED ]->S3 - noop
S4->[ ALTER TABLE ... ALTER CONSTRAINT ... ENFORCED ]->S4 - noop
S1->[ ALTER TABLE ... VALIDATE CONSTRAINT ... ]->S1 - but this is not
noop - the existing data gets validated but no change happens to the
state of the constraint - it is not enforced on the future data and
it's not considered valid. This gives opportunity to the user to just
validate the existing data but not enforce the constraint on new data
thus avoiding some computation on the new data. Of course we will have
to update the documentation to clearly specify the result. I think
VALIDATE CONSTRAINT is postgresql extension so we are free to
interpret it in the context of ENFORCED feature.
S3->[ ALTER TABLE ... VALIDATE CONSTRAINT ... ]->S4
S4->[ ALTER TABLE ... VALIDATE CONSTRAINT ... ]->S4 - noop
S1-[ ALTER TABLE ... ALTER CONSTRAINT ... NOT ENFORCED ]->S1 - noop
S3-[ ALTER TABLE ... ALTER CONSTRAINT ... NOT ENFORCED ]->S1
S4-[[ ALTER TABLE ... ALTER CONSTRAINT ... NOT ENFORCED ]->S1
Notice that there are no edges to and from S2.
--
Best Wishes,
Ashutosh Bapat
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2025-02-13 05:34 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-08-14 17:26 [PATCH v2] pg_dump: fix dependencies on FKs to multi-level partitioned tables Alvaro Herrera <[email protected]>
2025-02-12 14:45 Re: NOT ENFORCED constraint feature Peter Eisentraut <[email protected]>
2025-02-13 05:34 ` Re: NOT ENFORCED constraint feature Ashutosh Bapat <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox