Re: addFkRecurseReferencing use unassigned fkconstraint->fk_with_period value

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