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
Attachments:
0001-Fixups-remove-empty-line-fix-oversight-of-ca87c41.patchapplication/octet-stream; name=0001-Fixups-remove-empty-line-fix-oversight-of-ca87c41.patchDownload
From 3de8f2e12edbd9660ea9f308023ca8ceb22d8d74 Mon Sep 17 00:00:00 2001
From: reshke <reshke@yezzey-cbdb-bench.ru-central1.internal>
Date: Fri, 28 Mar 2025 16:35:57 +0000
Subject: [PATCH] Fixups: remove empty line & fix oversight of ca87c41
---
src/backend/optimizer/util/plancat.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 0489ad36644..a96ccd2a3a1 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -1299,8 +1299,7 @@ get_relation_constraints(PlannerInfo *root,
/*
* 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.
+ * ignore it here.
*/
if (!constr->check[i].ccvalid)
continue;
@@ -1317,7 +1316,6 @@ get_relation_constraints(PlannerInfo *root,
if (constr->check[i].ccnoinherit && !include_noinherit)
continue;
-
cexpr = stringToNode(constr->check[i].ccbin);
/*
--
2.43.0
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