On Wed, Apr 2, 2025 at 6:02 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 28.03.25 14:27, Amul Sul wrote:
> > On Fri, Mar 28, 2025 at 3:34 PM Ashutosh Bapat
> > <ashutosh.bapat.oss@gmail.com> wrote:
> >>
> >> On Thu, Mar 27, 2025 at 6:25 PM Amul Sul <sulamul@gmail.com> wrote:
> >>
> >>>
> >>> I am not sure how to make such tests stable enough since the trigger
> >>> name involves OIDs. In count check, I tried adjusting the join
> >>> condition to ensure that I get the exact same type of constraint
> >>> w.r.t. trigger relation and the constraint.
> >>
> >> There are tests which mask variable parts of EXPLAIN output. Can we
> >> use similar trick to mask OIDs from the trigger names?
> >
> > Okay, tried it in the attached version. Please check if it looks good.
>
> I have committed version 21 of the patches (without 0006).
>
> The patch you posted failed the regression test foreign_key because in
> the output of the queries that list the triggers, the conname output did
> not match the expected output. I committed it so that the test output
> matches the code behavior. But please double-check that that's what you
> intended.
>
Interestingly, it's not failing for me, and what's even stranger is
that the version with the committed test works fine on my system as
well. :)
> Also, something we hadn't looked at before, I think, I made
> get_relation_foreign_keys() in src/backend/optimizer/util/plancat.c
> ignore not-enforced foreign keys. That means, not-enforced foreign keys
> will not be used for cost estimation. This is, I think, what we want,
> as we discussed earlier. If we ever want an alternative mode where
> not-enforced constraints are considered for cost-estimation, then we
> could quite easily tweak this.
>
Yeah, it makes sense.
Thanks so much for the review, committing the patch, and all your guidance.
Regards,
Amul
