Thread: duplicated comments on get_relation_constraints
hi.
in plancat.c, function: get_relation_constraints
```
for (i = 0; i < num_check; i++)
{
Node *cexpr;
/*
* If this constraint hasn't been fully validated yet, we must
* ignore it here. Also ignore if NO INHERIT and we weren't told
* that that's safe.
*/
if (!constr->check[i].ccvalid)
continue;
/*
* NOT ENFORCED constraints are always marked as invalid, which
* should have been ignored.
*/
Assert(constr->check[i].ccenforced);
/*
* Also ignore if NO INHERIT and we weren't told that that's safe.
*/
if (constr->check[i].ccnoinherit && !include_noinherit)
continue;
}
``
The first "Also ignore if NO INHERIT and we weren't told that that's
safe." is duplicated,
also it's in the wrong place?
The second one is fine.
On Fri, 28 Mar 2025 at 09:47, jian he <jian.universality@gmail.com> wrote:
>
> hi.
>
> in plancat.c, function: get_relation_constraints
>
> ```
> for (i = 0; i < num_check; i++)
> {
> Node *cexpr;
> /*
> * If this constraint hasn't been fully validated yet, we must
> * ignore it here. Also ignore if NO INHERIT and we weren't told
> * that that's safe.
> */
> if (!constr->check[i].ccvalid)
> continue;
>
> /*
> * NOT ENFORCED constraints are always marked as invalid, which
> * should have been ignored.
> */
> Assert(constr->check[i].ccenforced);
>
> /*
> * Also ignore if NO INHERIT and we weren't told that that's safe.
> */
> if (constr->check[i].ccnoinherit && !include_noinherit)
> continue;
> }
> ``
>
> The first "Also ignore if NO INHERIT and we weren't told that that's
> safe." is duplicated,
> also it's in the wrong place?
> The second one is fine.
>
>
Hi! Indeed. Looks like an oversight from ca87c41. I think we can
safely remove one of those, presumably the first one.
--
Best regards,
Kirill Reshke
On Fri, Mar 28, 2025 at 4:53 PM Kirill Reshke <reshkekirill@gmail.com> wrote: > On Fri, 28 Mar 2025 at 09:47, jian he <jian.universality@gmail.com> wrote: > > The first "Also ignore if NO INHERIT and we weren't told that that's > > safe." is duplicated, > > also it's in the wrong place? > > The second one is fine. > Hi! Indeed. Looks like an oversight from ca87c41. I think we can > safely remove one of those, presumably the first one. +1. Also there is an extra blank line after the NO INHERIT check. I think we can remove it while we're here. Thanks Richard
On Fri, 28 Mar 2025 at 15:19, Richard Guo <guofenglinux@gmail.com> wrote: > > On Fri, Mar 28, 2025 at 4:53 PM Kirill Reshke <reshkekirill@gmail.com> wrote: > > On Fri, 28 Mar 2025 at 09:47, jian he <jian.universality@gmail.com> wrote: > > > The first "Also ignore if NO INHERIT and we weren't told that that's > > > safe." is duplicated, > > > also it's in the wrong place? > > > The second one is fine. > > > Hi! Indeed. Looks like an oversight from ca87c41. I think we can > > safely remove one of those, presumably the first one. > > +1. Also there is an extra blank line after the NO INHERIT check. I > think we can remove it while we're here. > > Thanks > Richard Sure. PFA attached, if needed -- Best regards, Kirill Reshke
Attachment
On Sat, Mar 29, 2025 at 1:38 AM Kirill Reshke <reshkekirill@gmail.com> wrote: > On Fri, 28 Mar 2025 at 15:19, Richard Guo <guofenglinux@gmail.com> wrote: > > On Fri, Mar 28, 2025 at 4:53 PM Kirill Reshke <reshkekirill@gmail.com> wrote: > > > On Fri, 28 Mar 2025 at 09:47, jian he <jian.universality@gmail.com> wrote: > > > > The first "Also ignore if NO INHERIT and we weren't told that that's > > > > safe." is duplicated, > > > > also it's in the wrong place? > > > > The second one is fine. > > > Hi! Indeed. Looks like an oversight from ca87c41. I think we can > > > safely remove one of those, presumably the first one. > > +1. Also there is an extra blank line after the NO INHERIT check. I > > think we can remove it while we're here. > Sure. PFA attached, if needed The change looks good to me. I plan to push it soon, barring any objections. Thanks Richard
On Wed, Apr 2, 2025 at 11:39 AM Richard Guo <guofenglinux@gmail.com> wrote: > The change looks good to me. I plan to push it soon, barring any > objections. Pushed. Thanks Richard