Suspicious redundant assignment in COPY FROM
Hi all, I was reading code of COPY FROM and I found some suspicious
redundant assignment for tuple descriptor and number of attributes. Is
it a behavior on purpose, or an accidently involved by the refactor in
c532d15? Patch is attached.
Attachments:
0001-Remove-redundant-assignment-in-copyfrom.patchapplication/octet-stream; name=0001-Remove-redundant-assignment-in-copyfrom.patchDownload
From a26c9da0edb229091e3b3f9eb0c62cd62c50f7be Mon Sep 17 00:00:00 2001
From: Jingtang Zhang <mrdrivingduck@gmail.com>
Date: Fri, 8 Sep 2023 11:42:07 +0800
Subject: [PATCH] Remove redundant assignment in copyfrom
There is redundant assignment of tuple descriptor and number of
attributes in BeginCopyFrom, which I believe is involved due to COPY
refactor in c532d15.
---
src/backend/commands/copyfrom.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index b47cb5c66d..8597021d10 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1383,14 +1383,13 @@ BeginCopyFrom(ParseState *pstate,
cstate->rel = rel;
tupDesc = RelationGetDescr(cstate->rel);
+ num_phys_attrs = tupDesc->natts;
/* process common options or initialization */
/* Generate or convert list of attributes to process */
cstate->attnumlist = CopyGetAttnums(tupDesc, cstate->rel, attnamelist);
- num_phys_attrs = tupDesc->natts;
-
/* Convert FORCE_NOT_NULL name list to per-column flags, check validity */
cstate->opts.force_notnull_flags = (bool *) palloc0(num_phys_attrs * sizeof(bool));
if (cstate->opts.force_notnull)
@@ -1533,8 +1532,6 @@ BeginCopyFrom(ParseState *pstate,
cstate->rteperminfos = pstate->p_rteperminfos;
}
- tupDesc = RelationGetDescr(cstate->rel);
- num_phys_attrs = tupDesc->natts;
num_defaults = 0;
volatile_defexprs = false;
--
2.39.2 (Apple Git-143)
On Fri, Sep 08, 2023 at 12:23:17PM +0800, Jingtang Zhang wrote:
Hi all, I was reading code of COPY FROM and I found some suspicious
redundant assignment for tuple descriptor and number of attributes. Is
it a behavior on purpose, or an accidently involved by the refactor in
c532d15? Patch is attached.
This looks like a copy-pasto to me, as the tuple descriptor coming
from the relation is just used for sanity checks on the attributes
depending on the options by the caller for the COPY.
The assignment of num_phys_attrs could be kept at the same place as
on HEAD, a bit closer to the palloc0() where it is used.
--
Michael
Michael Paquier <michael@paquier.xyz> 于2023年9月8日周五 13:42写道：
Thanks, Michael~
The assignment of num_phys_attrs could be kept at the same place as
on HEAD, a bit closer to the palloc0() where it is used.
Agreed with this principle. Patch is modified and attached.
--
Jingtang
Attachments:
v2-0001-Remove-redundant-assignment-in-copyfrom.patchapplication/octet-stream; name=v2-0001-Remove-redundant-assignment-in-copyfrom.patchDownload
From 1c37980cc84af129fd9f35bb9fdaf577d36217b8 Mon Sep 17 00:00:00 2001
From: Jingtang Zhang <mrdrivingduck@gmail.com>
Date: Fri, 8 Sep 2023 13:48:40 +0800
Subject: [PATCH v2] Remove redundant assignment in copyfrom
There is redundant assignment of tuple descriptor and number of
attributes in BeginCopyFrom, which I believe is involved due to COPY
refactor in c532d15.
---
src/backend/commands/copyfrom.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index b47cb5c66d..70871ed819 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1533,8 +1533,6 @@ BeginCopyFrom(ParseState *pstate,
cstate->rteperminfos = pstate->p_rteperminfos;
}
- tupDesc = RelationGetDescr(cstate->rel);
- num_phys_attrs = tupDesc->natts;
num_defaults = 0;
volatile_defexprs = false;
--
2.39.2 (Apple Git-143)