From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
---|---|
To: | vignesh C <vignesh21(at)gmail(dot)com> |
Cc: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Cleanup - Removal of unused function parameter from CopyReadBinaryAttribute |
Date: | 2020-06-19 05:29:38 |
Message-ID: | CALj2ACW+bVF8oP1xyoPFx3Fp8Rk=y533D6Nk24E+dLoKU5egWA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> > > 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. If we want to use column_no in the log message again,
> > it's better to check why commit 0e319c7ad7 got rid of column_no from
> > the message.
>
> I noticed that displaying of column information is present and it is
> done in a different way. Basically cstate->cur_attname is set with the
> column name before calling CopyReadBinaryAttribute function. If there
> is any error in CopyReadBinaryAttribute function,
> CopyFromErrorCallback will be called. CopyFromErrorCallback function
> takes care of displaying the column name by using cstate->cur_attname.
> I tried simulating this and it displays the column name neatly in the
> error message.:
> postgres=# copy t1 from '/home/db/copydata/t1_copy.bin' with (format 'binary');
> ERROR: invalid field size
> CONTEXT: COPY t1, line 1, column c1
> I feel we can safely remove the parameter as in the patch.
>
thanks for this information.
+1 for this patch.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Kyotaro Horiguchi | 2020-06-19 05:35:11 | Re: min_safe_lsn column in pg_replication_slots view |
Previous Message | David Rowley | 2020-06-19 05:27:13 | Re: Missing HashAgg EXPLAIN ANALYZE details for parallel plans |