Hi,
Haibo Yan wrote:
> The fix is to set conwithperiod from the correct source:
I tested v1 on 3c5d9d914f with assertions enabled on macos.
without_overlaps and the full regress and isolation suites pass.
With only the tablecmds.c changes reverted, the new tests fail at all
four expected rejection cases: VALIDATE, ENFORCED, and both ATTACH
paths.
The changes look right to me. In addFkRecurseReferencing(), with_period
is correct for both the parser path and the partially reconstructed
Constraint from CloneFkReferencing(). The other two assignments take it
from the catalog row being validated. I found no other producer of a
foreign-key NewConstraint.
I tried multiranges with gaps in coverage, partitioned tables on both
sides of the FK, recursive validation and enforcement, attaching a
partitioned table with a different column order, and reusing a NOT VALID
FK during ADD FOREIGN KEY. The affected operations accept uncovered
rows on the base revision and reject them with v1; the corresponding
covered rows pass with v1.
One point for the commit message: constraints that went through one of
these paths on 18.x may already be marked valid with uncovered rows.
The fix does not recheck them, and VALIDATE CONSTRAINT is a no-op for
a valid constraint. Such rows can be found with a query modeled on the
RI check; alternatively, with the fix, the constraint can be dropped and
re-added as NOT VALID in one transaction and then validated separately,
which keeps it enforced for new rows if validation fails.
For a separate cleanup, RI_Initial_Check() could return false when
riinfo->hasperiod is true. It already fetches that information, so
this could avoid making callers maintain another copy of the flag.
I think the minimal fix in v1 is suitable for backpatching as it stands.
Two small test nits: "with pre-existing rows" would read better than
"with rows already", and the NOT VALID/NOT ENFORCED block would fit
better after the pg_get_constraintdef checks, keeping those checks
next to the constraint they inspect.
I think this is ready for a committer.
Best regards,
Andrew Krylosov
