Re: Cleanup - Removal of unused function parameter from CopyReadBinaryAttribute - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: Cleanup - Removal of unused function parameter from CopyReadBinaryAttribute
Date
Msg-id CAA4eK1JmtbX+SzwwYEc4txf9oyGPYQGL8Jo03W2E7AeOwM1m=g@mail.gmail.com
Whole thread Raw
In response to Re: Cleanup - Removal of unused function parameter fromCopyReadBinaryAttribute  (Fujii Masao <masao.fujii@oss.nttdata.com>)
Responses Re: Cleanup - Removal of unused function parameter from CopyReadBinaryAttribute
List pgsql-hackers
On Thu, Jun 18, 2020 at 10:05 PM Fujii Masao
<masao.fujii@oss.nttdata.com> wrote:
>
>
> On 2020/06/18 23:09, Bharath Rupireddy wrote:
> > On Thu, Jun 18, 2020 at 7:01 PM vignesh C <vignesh21@gmail.com> wrote:
> >>
> >> Hi,
> >>
> >> While checking copy from code I found that the function parameter
> >> column_no is not used in CopyReadBinaryAttribute. I felt this could be
> >> removed.
> >> Attached patch contains the changes for the same.
> >> Thoughts?
> >>
> >
> > I don't see any problem in removing this extra parameter.
> >
> > However another thought, can it be used to report a bit meaningful
> > error for field size < 0 check?
>
> column_no was used for that purpose in the past, but commit 0e319c7ad7
> changed that.
>

Yeah, but not sure why?  By looking at the commit message and change
it is difficult to say why it has been removed?  Tom has made that
change but I don't think he would remember it, in any case, adding him
in the email to see if he remembers anything related to it.

commit 0e319c7ad7665673103f0b10752700fd2f33acd3
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date:   Mon Sep 29 22:06:40 2003 +0000

    Improve context display for failures during COPY IN, as recently
    discussed on pghackers.
..
..
@@ -1917,7 +2019,7 @@ CopyReadBinaryAttribute(int column_no, FmgrInfo
*flinfo, Oid typelem,
  if (fld_size < 0)
  ereport(ERROR,
  (errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
- errmsg("invalid size for field %d", column_no)));
+ errmsg("invalid field size")));

  /* reset attribute_buf to empty, and load raw data in it */
  attribute_buf.len = 0;
@@ -1944,8 +2046,7 @@ CopyReadBinaryAttribute(int column_no, FmgrInfo
*flinfo, Oid typelem,
  if (attribute_buf.cursor != attribute_buf.len)
  ereport(ERROR,
  (errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
- errmsg("incorrect binary data format in field %d",
- column_no)));
+ errmsg("incorrect binary data format")));

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com



pgsql-hackers by date:

Previous
From: Justin Pryzby
Date:
Subject: Re: Missing HashAgg EXPLAIN ANALYZE details for parallel plans
Next
From: David Rowley
Date:
Subject: Re: Missing HashAgg EXPLAIN ANALYZE details for parallel plans