On Mon, Dec 16, 2024 at 7:50 PM Nishant Sharma
<nishant.sharma@enterprisedb.com> wrote:
>
>
> 1) The new switch logic does not look correct to me. It will pass for
> a failing scenario. I think you can use v3's logic instead with below
> changes:-
>
> a)
> while (HeapTupleIsValid(atup = systable_getnext(ascan))) -->
> while (HeapTupleIsValid(atup = systable_getnext(ascan)) && on_error_tbl_ok)
>
> b)
> attcnt++; --> just before the "switch (attForm->attnum)".
>
> Thats it.
>
You are right about this.
On Tue, Dec 17, 2024 at 12:31 PM Kirill Reshke <reshkekirill@gmail.com> wrote:
>
> On Mon, 16 Dec 2024 at 16:50, Nishant Sharma
> <nishant.sharma@enterprisedb.com> wrote:
> > Also, I think Andrew's suggestion can resolve the concern me and Krill
> > had on forcing users to create tables with correct column names and
> > numbers. Also, will make error table checking simpler. No need for the
> > above kind of checks.
>
> +1 on that.
>
Syntax: COPY (on_error table, table error_saving_tbl);
seems not ideal.
but auto-create on_error table if this table does not exist, seems way
more harder.
Since we can not use SPI interface here, maybe we can use DefineRelation
also, to auto-create a table, what if table already exists, then
our operation would be stuck for not COPY related reason.
also auto-create means we need to come up with a magic table name for
all COPY (on_error table)
operations, which seems not ideal IMO.
i realized we should error out case like:
COPY err_tbl FROM STDIN WITH (DELIMITER ',', on_error table, table err_tbl);
also by changing copy_generic_opt_arg, now we can
COPY err_tbl FROM STDIN WITH (DELIMITER ',', on_error table, table x);
previously, we can only do
COPY err_tbl FROM STDIN WITH (DELIMITER ',', on_error 'table', table x);