ecpg command does not warn COPY ... FROM STDIN;

Lists: pgsql-hackers
From: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-08 12:41:03
Message-ID: CANOn0Ez_t5uDCUEV8c1YORMisJiU5wu681eEVZzgKwOeiKhkqQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi hackers,

I found a code validation bug in master branch.

Now, ecpg does not support 'EXEC SQL COPY ... FROM STDIN ... ;' and
code for warning it exits.

https://github.com/postgres/postgres/blob/7b27f5fd36cb3270e8ac25aefd73b552663d1392/src/interfaces/ecpg/preproc/ecpg.addons#L242-L245
---
ECPG: addon CopyStmt COPY opt_binary qualified_name opt_column_list
copy_from opt_program copy_file_name copy_delimiter opt_with
copy_options where_clause
if (strcmp(@6, "from") == 0 &&
(strcmp(@7, "stdin") == 0 || strcmp(@7, "stdout") == 0))
mmerror(PARSE_ERROR, ET_WARNING, "COPY FROM STDIN is not implemented");
---

But it is not working.
ecpg command fails to notice though code like above exits on pgc code.

# COPY ... FROM STDIN code
ryo(at)DESKTOP-IOASPN6:~/work/postgres/src$ cat copy_from_should_be_warned.pgc
#include <stdio.h>
#include <stdlib.h>

EXEC SQL WHENEVER SQLERROR CALL die();
EXEC SQL WHENEVER NOT FOUND DO BREAK;

void
die(void)
{
fprintf(stderr, "%s\n", sqlca.sqlerrm.sqlerrmc);
exit(1);
}

int
main(void)
{
EXEC SQL BEGIN DECLARE SECTION;
const char *target = "postgres(at)localhost:5432";
const char *user = "ryo";
const char *passwd = "";
EXEC SQL END DECLARE SECTION;

EXEC SQL CONNECT TO :target USER :user USING :passwd;
EXEC SQL COPY name_age_list FROM STDIN;
EXEC SQL COMMIT;
EXEC SQL DISCONNECT ALL;

return 0;
}
-----

I executed ecpg command for above code.

ryo(at)DESKTOP-IOASPN6:~/work/postgres/src$ ../master/bin/ecpg
copy_from_should_be_warned.pgc
ryo(at)DESKTOP-IOASPN6:~/work/postgres/src$

But there was no warning and generaged c source.

So, I wrote patch.
the patch changes @6 on code above to @5.
Checked variable was wrong.

After apply patch, warning is shown.
(c source is generated as before)

ryo(at)DESKTOP-IOASPN6:~/work/postgres/src$ ../master/bin/ecpg
copy_from_should_be_warned.pgc
copy_from_should_be_warned.pgc:24: WARNING: COPY FROM STDIN is not implemented
ryo(at)DESKTOP-IOASPN6:~/work/postgres/src$

--
Best regards,
Ryo Kanbayashi
kanbayashi(dot)dev(at)gmail(dot)com
https://github.com/ryogrid

Attachment Content-Type Size
patch_checking_note.txt text/plain 8.1 KB
copy_from_ok.pgc application/octet-stream 610 bytes
copy_from_should_be_warned.pgc application/octet-stream 576 bytes
copy_from_stdin_no_warning.patch application/octet-stream 851 bytes

From: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Ryo Kanbayashi' <kanbayashi(dot)dev(at)gmail(dot)com>
Cc: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-08 13:35:51
Message-ID: OS7PR01MB149680266F200CC96D0EF3FC3F5122@OS7PR01MB14968.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Dear Kanbayashi-san,

> I found a code validation bug in master branch.
>
> Now, ecpg does not support 'EXEC SQL COPY ... FROM STDIN ... ;' and
> code for warning it exits.
>
> https://github.com/postgres/postgres/blob/7b27f5fd36cb3270e8ac25aefd73b55
> 2663d1392/src/interfaces/ecpg/preproc/ecpg.addons#L242-L245
> ---
> ECPG: addon CopyStmt COPY opt_binary qualified_name opt_column_list
> copy_from opt_program copy_file_name copy_delimiter opt_with
> copy_options where_clause
> if (strcmp(@6, "from") == 0 &&
> (strcmp(@7, "stdin") == 0 || strcmp(@7, "stdout") == 0))
> mmerror(PARSE_ERROR, ET_WARNING, "COPY FROM STDIN is not
> implemented");
> ---
>
> But it is not working.
> ecpg command fails to notice though code like above exits on pgc code.

Good catch. I have a comment about the fix.

The parser accepts a statement like "COPY ... FROM STDOUT", and ISTM it is not either
implemented yet. However, the warning message only mentions STDIN case even STDOUT
is specified.

EXEC SQL COPY foo FROM STDOUT;
->
WARNING: COPY FROM STDIN is not implemented

I feel we can change like "COPY FROM STDIN/STDOUT...", or prepare two messages.
Thought?

Best regards,
Hayato Kuroda
FUJITSU LIMITED


From: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>
To: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-08 14:04:18
Message-ID: CANOn0EwOgYcB-NmanjsGSeo7WE4tNCO=sp2C7wT97KhMucUqUg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Kuroda-san, thank to your comment.

> > I found a code validation bug in master branch.
> >
> > Now, ecpg does not support 'EXEC SQL COPY ... FROM STDIN ... ;' and
> > code for warning it exits.
> >
> > https://github.com/postgres/postgres/blob/7b27f5fd36cb3270e8ac25aefd73b55
> > 2663d1392/src/interfaces/ecpg/preproc/ecpg.addons#L242-L245
> > ---
> > ECPG: addon CopyStmt COPY opt_binary qualified_name opt_column_list
> > copy_from opt_program copy_file_name copy_delimiter opt_with
> > copy_options where_clause
> > if (strcmp(@6, "from") == 0 &&
> > (strcmp(@7, "stdin") == 0 || strcmp(@7, "stdout") == 0))
> > mmerror(PARSE_ERROR, ET_WARNING, "COPY FROM STDIN is not
> > implemented");
> > ---
> >
> > But it is not working.
> > ecpg command fails to notice though code like above exits on pgc code.
>
> Good catch. I have a comment about the fix.
>
> The parser accepts a statement like "COPY ... FROM STDOUT", and ISTM it is not either
> implemented yet. However, the warning message only mentions STDIN case even STDOUT
> is specified.
>
> EXEC SQL COPY foo FROM STDOUT;
> ->
> WARNING: COPY FROM STDIN is not implemented
>
> I feel we can change like "COPY FROM STDIN/STDOUT...", or prepare two messages.
> Thought?

I think your proposed fix is better too.
So, I modified the patch.

With new patch, warning message is changed like below :)

ryo(at)DESKTOP-IOASPN6:~/work/postgres/src$ ../master/bin/ecpg
copy_from_should_be_warned.pgc
copy_from_should_be_warned.pgc:24: WARNING: COPY FROM STDIN/STDOUT is
not implemented
ryo(at)DESKTOP-IOASPN6:~/work/postgres/src$

--
Best regards,
Ryo Kanbayashi
https://github.com/ryogrid

On Wed, Jan 8, 2025 at 10:35 PM Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> Dear Kanbayashi-san,
>
> > I found a code validation bug in master branch.
> >
> > Now, ecpg does not support 'EXEC SQL COPY ... FROM STDIN ... ;' and
> > code for warning it exits.
> >
> > https://github.com/postgres/postgres/blob/7b27f5fd36cb3270e8ac25aefd73b55
> > 2663d1392/src/interfaces/ecpg/preproc/ecpg.addons#L242-L245
> > ---
> > ECPG: addon CopyStmt COPY opt_binary qualified_name opt_column_list
> > copy_from opt_program copy_file_name copy_delimiter opt_with
> > copy_options where_clause
> > if (strcmp(@6, "from") == 0 &&
> > (strcmp(@7, "stdin") == 0 || strcmp(@7, "stdout") == 0))
> > mmerror(PARSE_ERROR, ET_WARNING, "COPY FROM STDIN is not
> > implemented");
> > ---
> >
> > But it is not working.
> > ecpg command fails to notice though code like above exits on pgc code.
>
> Good catch. I have a comment about the fix.
>
> The parser accepts a statement like "COPY ... FROM STDOUT", and ISTM it is not either
> implemented yet. However, the warning message only mentions STDIN case even STDOUT
> is specified.
>
> EXEC SQL COPY foo FROM STDOUT;
> ->
> WARNING: COPY FROM STDIN is not implemented
>
> I feel we can change like "COPY FROM STDIN/STDOUT...", or prepare two messages.
> Thought?
>
> Best regards,
> Hayato Kuroda
> FUJITSU LIMITED
>

Attachment Content-Type Size
copy_from_stdin_no_warning2.patch application/octet-stream 1023 bytes

From: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
To: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-08 15:19:37
Message-ID: [email protected]
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2025/01/08 23:04, Ryo Kanbayashi wrote:
>>> But it is not working.
>>> ecpg command fails to notice though code like above exits on pgc code.

This issue seems to have been introduced in commit 3d009e45bd. Before that,
as per my testing, ecpg successfully detected COPY FROM STDIN and reported
a warning. It seems the fix will need to be back-patched to all supported versions.

>> I feel we can change like "COPY FROM STDIN/STDOUT...", or prepare two messages.
>> Thought?
>
> I think your proposed fix is better too.
> So, I modified the patch.

As for COPY FROM STDOUT, while it's possible, mentioning it might be
confusing since it's not official syntax. So I'm not sure if this is
good idea.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Cc: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-08 15:42:09
Message-ID: [email protected]
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> writes:
> On 2025/01/08 23:04, Ryo Kanbayashi wrote:
> But it is not working.
> ecpg command fails to notice though code like above exits on pgc code.

> This issue seems to have been introduced in commit 3d009e45bd.

Indeed :-(

> As for COPY FROM STDOUT, while it's possible, mentioning it might be
> confusing since it's not official syntax. So I'm not sure if this is
> good idea.

There's another problem: the correct syntax is COPY TO STDOUT,
and that won't trigger this warning either.

I'm inclined to drop the test on @5 altogether, and just check for
"stdin" or "stdout" in @7. There is no variant of those that will
work. (But maybe we should allow it if opt_program isn't empty?)

The warning message could perhaps be written
"COPY FROM STDIN/TO STDOUT is not implemented".

regards, tom lane


From: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-08 16:04:40
Message-ID: [email protected]
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2025/01/09 0:42, Tom Lane wrote:
> Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> writes:
>> On 2025/01/08 23:04, Ryo Kanbayashi wrote:
>> But it is not working.
>> ecpg command fails to notice though code like above exits on pgc code.
>
>> This issue seems to have been introduced in commit 3d009e45bd.
>
> Indeed :-(
>
>> As for COPY FROM STDOUT, while it's possible, mentioning it might be
>> confusing since it's not official syntax. So I'm not sure if this is
>> good idea.
>
> There's another problem: the correct syntax is COPY TO STDOUT,
> and that won't trigger this warning either.

ISTM that ecpg supports COPY TO STDOUT and includes the regression test "copystdout" for it. No?

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Cc: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-08 16:31:10
Message-ID: [email protected]
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> writes:
> On 2025/01/09 0:42, Tom Lane wrote:
>> There's another problem: the correct syntax is COPY TO STDOUT,
>> and that won't trigger this warning either.

> ISTM that ecpg supports COPY TO STDOUT and includes the regression test "copystdout" for it. No?

Oh right. (Pokes at it...) It looks like the backend accepts
"FROM STDOUT" as a synonym for "FROM STDIN", so that's why this
is checking for both spellings. But I agree it'd be better
for the error message to only use the standard spelling.

Also I tried

regression=# copy int4_tbl from program stdin;
ERROR: STDIN/STDOUT not allowed with PROGRAM

So we probably don't need to bother with adjusting the check
to allow that.

regards, tom lane


From: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Tom Lane' <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Cc: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-09 07:52:57
Message-ID: OSCPR01MB14966060B76E328E491D39F1EF5132@OSCPR01MB14966.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Dear Tom, Fujii-san,

> > ISTM that ecpg supports COPY TO STDOUT and includes the regression test
> "copystdout" for it. No?
>
> Oh right. (Pokes at it...) It looks like the backend accepts
> "FROM STDOUT" as a synonym for "FROM STDIN", so that's why this
> is checking for both spellings. But I agree it'd be better
> for the error message to only use the standard spelling.
>
> Also I tried
>
> regression=# copy int4_tbl from program stdin;
> ERROR: STDIN/STDOUT not allowed with PROGRAM
>
> So we probably don't need to bother with adjusting the check
> to allow that.

To confirm - I have no objections for the decision. I'm also think it is not
an usual grammar. I checked the doc [1] and I could not find "COPY FROM STDOUT".
I.e., first version is enough.

[1]: https://www.postgresql.org/docs/devel/sql-copy.html

Best regards,
Hayato Kuroda
FUJITSU LIMITED


From: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>
To: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-09 11:34:54
Message-ID: CANOn0Ex9oN_ZFm9v8Sf4X6PSMkh7buoo2713wkLgGHKUXMLcpg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Dear Tom, Fujii-san, Kuroda-san,

I saw comments of yours and recognized that better fix is below.

- Fix of first attached patch which does not change warning message

And I created patch entry of commitfest :)
https://commitfest.postgresql.org/52/5497/

What should I do additionally?
Do I need to write patches for current supporting versions? (12.x - 17.x)

--
Best regards,
Ryo Kanbayashi
https://github.com/ryogrid

On Thu, Jan 9, 2025 at 4:53 PM Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> Dear Tom, Fujii-san,
>
> > > ISTM that ecpg supports COPY TO STDOUT and includes the regression test
> > "copystdout" for it. No?
> >
> > Oh right. (Pokes at it...) It looks like the backend accepts
> > "FROM STDOUT" as a synonym for "FROM STDIN", so that's why this
> > is checking for both spellings. But I agree it'd be better
> > for the error message to only use the standard spelling.
> >
> > Also I tried
> >
> > regression=# copy int4_tbl from program stdin;
> > ERROR: STDIN/STDOUT not allowed with PROGRAM
> >
> > So we probably don't need to bother with adjusting the check
> > to allow that.
>
> To confirm - I have no objections for the decision. I'm also think it is not
> an usual grammar. I checked the doc [1] and I could not find "COPY FROM STDOUT".
> I.e., first version is enough.
>
> [1]: https://www.postgresql.org/docs/devel/sql-copy.html
>
> Best regards,
> Hayato Kuroda
> FUJITSU LIMITED
>

Attachment Content-Type Size
copy_from_stdin_no_warning.patch application/octet-stream 851 bytes

From: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
To: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-09 11:59:42
Message-ID: [email protected]
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2025/01/09 20:34, Ryo Kanbayashi wrote:
> Dear Tom, Fujii-san, Kuroda-san,
>
> I saw comments of yours and recognized that better fix is below.
>
> - Fix of first attached patch which does not change warning message
>
> And I created patch entry of commitfest :)
> https://commitfest.postgresql.org/52/5497/
>
> What should I do additionally?
> Do I need to write patches for current supporting versions? (12.x - 17.x)

Testing the patch across all supported versions would be helpful.
If adjustments are needed for specific versions, creating separate
patches for those would also be appreciated. Since v12 is no longer
supported, back-patching to it isn't necessary.

BTW, regarding the discussion on the list, please avoid top-posting;
bottom-posting is the preferred style on this mailing list.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION


From: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>
To: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-09 12:27:47
Message-ID: CANOn0EzoiQmkFoEEEiz-0+O8-SbK7Em+0Z5ibXY1CSmJZ9A3RA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

> On 2025/01/09 20:34, Ryo Kanbayashi wrote:
> > Dear Tom, Fujii-san, Kuroda-san,
> >
> > I saw comments of yours and recognized that better fix is below.
> >
> > - Fix of first attached patch which does not change warning message
> >
> > And I created patch entry of commitfest :)
> > https://commitfest.postgresql.org/52/5497/
> >
> > What should I do additionally?
> > Do I need to write patches for current supporting versions? (12.x - 17.x)
>
> Testing the patch across all supported versions would be helpful.
> If adjustments are needed for specific versions, creating separate
> patches for those would also be appreciated. Since v12 is no longer
> supported, back-patching to it isn't necessary.

thanks.
I try these :)

> BTW, regarding the discussion on the list, please avoid top-posting;
> bottom-posting is the preferred style on this mailing list.

I understand.
I'll be careful from now on :)

(Please Ignore: I attach renamed patch file for updating patch file on
commitfest system)

--
Best regards,
Ryo Kanbayashi
https://github.com/ryogrid

Attachment Content-Type Size
copy_from_stdin_no_warning_for_master.patch application/octet-stream 851 bytes

From: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>
To: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-10 08:45:00
Message-ID: CANOn0Ey8YbVZcemKzZEQ77v2WA+n2t+fmB6UQ+1-=A3fHqzUeg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Jan 9, 2025 at 9:27 PM Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com> wrote:
>
> > On 2025/01/09 20:34, Ryo Kanbayashi wrote:
> > > Dear Tom, Fujii-san, Kuroda-san,
> > >
> > > I saw comments of yours and recognized that better fix is below.
> > >
> > > - Fix of first attached patch which does not change warning message
> > >
> > > And I created patch entry of commitfest :)
> > > https://commitfest.postgresql.org/52/5497/
> > >
> > > What should I do additionally?
> > > Do I need to write patches for current supporting versions? (12.x - 17.x)
> >
> > Testing the patch across all supported versions would be helpful.
> > If adjustments are needed for specific versions, creating separate
> > patches for those would also be appreciated. Since v12 is no longer
> > supported, back-patching to it isn't necessary.
>
> thanks.
> I try these :)
>
> > BTW, regarding the discussion on the list, please avoid top-posting;
> > bottom-posting is the preferred style on this mailing list.
>
> I understand.
> I'll be careful from now on :)
>
> (Please Ignore: I attach renamed patch file for updating patch file on
> commitfest system)

PLEASE IGNORE THIS MAIL

(I attached wrong format patch file. So re-attach modified one for CFBot)

---
Great Reagrds,
Ryo Kanbayashi

Attachment Content-Type Size
copy_from_stdin_no_warning_for_master2.patch application/octet-stream 720 bytes

From: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>
To: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-10 10:34:49
Message-ID: CANOn0Ez85ow5kfKQ8mH4gtpE6-fMkhMLr1QzNjV333cHq5Uu8w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, Jan 10, 2025 at 5:45 PM Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com> wrote:
>
> On Thu, Jan 9, 2025 at 9:27 PM Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com> wrote:
> >
> > > On 2025/01/09 20:34, Ryo Kanbayashi wrote:
> > > > Dear Tom, Fujii-san, Kuroda-san,
> > > >
> > > > I saw comments of yours and recognized that better fix is below.
> > > >
> > > > - Fix of first attached patch which does not change warning message
> > > >
> > > > And I created patch entry of commitfest :)
> > > > https://commitfest.postgresql.org/52/5497/
> > > >
> > > > What should I do additionally?
> > > > Do I need to write patches for current supporting versions? (12.x - 17.x)
> > >
> > > Testing the patch across all supported versions would be helpful.
> > > If adjustments are needed for specific versions, creating separate
> > > patches for those would also be appreciated. Since v12 is no longer
> > > supported, back-patching to it isn't necessary.
> >
> > thanks.
> > I try these :)
> >
> > > BTW, regarding the discussion on the list, please avoid top-posting;
> > > bottom-posting is the preferred style on this mailing list.
> >
> > I understand.
> > I'll be careful from now on :)

PLEASE IGNORE THIS MAIL
SORRY TO BOTHER YOU AGAIN...

(I have not attached correct format patch file yet. So re-attach
modified one for CFBot)

Great Reagrds,
Ryo Kanbayashi

Attachment Content-Type Size
copy_from_stdin_no_warning_for_master3.patch application/octet-stream 765 bytes

From: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>
To: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Subject: Re: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-11 17:04:22
Message-ID: CANOn0Ez+gAOCcEVqaFGsjU4Ls3vKxe=ui-qxbq3iCf9baHsqTg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

> On Thu, Jan 9, 2025 at 9:27 PM Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com> wrote:
>
> > On 2025/01/09 20:34, Ryo Kanbayashi wrote:
> > > Dear Tom, Fujii-san, Kuroda-san,
> > >
> > > I saw comments of yours and recognized that better fix is below.
> > >
> > > - Fix of first attached patch which does not change warning message
> > >
> > > And I created patch entry of commitfest :)
> > > https://commitfest.postgresql.org/52/5497/
> > >
> > > What should I do additionally?
> > > Do I need to write patches for current supporting versions? (12.x - 17.x)
> >
> > Testing the patch across all supported versions would be helpful.
> > If adjustments are needed for specific versions, creating separate
> > patches for those would also be appreciated. Since v12 is no longer
> > supported, back-patching to it isn't necessary.
>
> thanks.
> I try these :)
>
> > BTW, regarding the discussion on the list, please avoid top-posting;
> > bottom-posting is the preferred style on this mailing list.
>
> I understand.
> I'll be careful from now on :)
>
> (Please Ignore: I attach renamed patch file for updating patch file on
> commitfest system)

I wrote a patch for release v13 - v17 additionally and tested it for
each release branch :)
As a result, two patch is needed for this fix.

copy_from_stdin_no_warning_for_master3.patch -> patch for master branch
copy_from_stdin_no_warning_for_stables.patch -> patch for v13 - v17

check_patches.sh -> utility script for testing above two patches on
each target branches
patch_checking_note_cf.txt -> checking result by check_patches.sh and etc
other files -> files which are needed for testing with check_patches.sh

[commitfest entry]
https://commitfest.postgresql.org/52/5497/

It would be helpful if someone could review patches I wrote :)

--
Best regards,
Ryo Kanbayashi
https://github.com/ryogrid

Attachment Content-Type Size
patch_checking_note_cf.txt text/plain 7.3 KB
copy_from_stdin_no_warning_for_master3.patch application/octet-stream 765 bytes
copy_from_stdin_no_warning_for_stables.patch application/octet-stream 767 bytes
check_patches.sh text/x-sh 5.2 KB
copy_from_should_be_warned.pgc application/octet-stream 576 bytes
copy_from_ok.pgc application/octet-stream 584 bytes

From: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
To: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-12 03:56:48
Message-ID: [email protected]
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2025/01/12 2:04, Ryo Kanbayashi wrote:
> I wrote a patch for release v13 - v17 additionally and tested it for
> each release branch :)
> As a result, two patch is needed for this fix.

Thanks for the patches! Barring any objections,
I plan to commit them with the following commit log.

-------------------
ecpg: Restore detection of unsupported COPY FROM STDIN.

The ecpg command includes code to warn about unsupported COPY FROM STDIN
statements in input files. However, since commit 3d009e45bd,
this functionality has been broken due to a bug introduced in that commit,
causing ecpg to fail to detect the statement.

This commit resolves the issue, restoring ecpg's ability to detect
COPY FROM STDIN and issue a warning as intended.

Back-patch to all supported versions.

Author: Ryo Kanbayashi
Reviewed-by: Hayato Kuroda, Tom Lane
Discussion: https://postgr.es/m/CANOn0Ez_t5uDCUEV8c1YORMisJiU5wu681eEVZzgKwOeiKhkqQ@mail.gmail.com
-------------------

> check_patches.sh -> utility script for testing above two patches on
> each target branches

Should we add a regression test to ensure ecpg correctly reports errors
and warnings, including the warning for COPY FROM STDIN? This could help
catch similar bugs more effectively. If agreed, we could create this
as a separate patch.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION


From: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>
To: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Cc: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-12 09:27:48
Message-ID: CANOn0Ex=YqrG5s9k8OwWoZHD_Q8oabJPaM2iLXMjJ82Uo7VX0g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Sun, Jan 12, 2025 at 12:56 PM Fujii Masao
<masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>
>
>
> On 2025/01/12 2:04, Ryo Kanbayashi wrote:
> > I wrote a patch for release v13 - v17 additionally and tested it for
> > each release branch :)
> > As a result, two patch is needed for this fix.
>
> Thanks for the patches! Barring any objections,
> I plan to commit them with the following commit log.
>
> -------------------
> ecpg: Restore detection of unsupported COPY FROM STDIN.
>
> The ecpg command includes code to warn about unsupported COPY FROM STDIN
> statements in input files. However, since commit 3d009e45bd,
> this functionality has been broken due to a bug introduced in that commit,
> causing ecpg to fail to detect the statement.
>
> This commit resolves the issue, restoring ecpg's ability to detect
> COPY FROM STDIN and issue a warning as intended.
>
> Back-patch to all supported versions.
>
> Author: Ryo Kanbayashi
> Reviewed-by: Hayato Kuroda, Tom Lane
> Discussion: https://postgr.es/m/CANOn0Ez_t5uDCUEV8c1YORMisJiU5wu681eEVZzgKwOeiKhkqQ@mail.gmail.com
> -------------------

Thank you for reviewing patch :)
The commit log matches with my recognition and has no problem.

> > check_patches.sh -> utility script for testing above two patches on
> > each target branches
>
> Should we add a regression test to ensure ecpg correctly reports errors
> and warnings, including the warning for COPY FROM STDIN? This could help
> catch similar bugs more effectively. If agreed, we could create this
> as a separate patch.

Of course there is no problem!
I think a test like above becomes a good regression test too.

--
Best regards,
Ryo Kanbayashi
https://github.com/ryogrid


From: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
To: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>
Cc: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-14 16:34:49
Message-ID: [email protected]
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2025/01/12 18:27, Ryo Kanbayashi wrote:
> Thank you for reviewing patch :)
> The commit log matches with my recognition and has no problem.

Pushed. Thanks!

>>> check_patches.sh -> utility script for testing above two patches on
>>> each target branches
>>
>> Should we add a regression test to ensure ecpg correctly reports errors
>> and warnings, including the warning for COPY FROM STDIN? This could help
>> catch similar bugs more effectively. If agreed, we could create this
>> as a separate patch.
>
> Of course there is no problem!
> I think a test like above becomes a good regression test too.

So, will you give creating the patch a try?

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION


From: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>
To: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Cc: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-17 08:46:57
Message-ID: CANOn0Ezh+aCaG-Z0b6D=6WWSr6UQA_vv762F3Z0Nj4Ag53fFZQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, Jan 15, 2025 at 1:34 AM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>
>
>
> On 2025/01/12 18:27, Ryo Kanbayashi wrote:
> > Thank you for reviewing patch :)
> > The commit log matches with my recognition and has no problem.
>
> Pushed. Thanks!
>
> >>> check_patches.sh -> utility script for testing above two patches on
> >>> each target branches
> >>
> >> Should we add a regression test to ensure ecpg correctly reports errors
> >> and warnings, including the warning for COPY FROM STDIN? This could help
> >> catch similar bugs more effectively. If agreed, we could create this
> >> as a separate patch.
> >
> > Of course there is no problem!
> > I think a test like above becomes a good regression test too.
>
> So, will you give creating the patch a try?

Yes. I try to write the patch for regression test of ecpg command
warning and error notice :)

BTW, How should we handle commit fest entry below?

"ecpg command does not warn COPY ... FROM STDIN;"
https://commitfest.postgresql.org/52/5497/

I think that the patch of regression test is not included the entry.

---
Great regards,
Ryo Kanbayashi
https://github.com/ryogrid


From: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
To: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>
Cc: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-17 13:51:49
Message-ID: [email protected]
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2025/01/17 17:46, Ryo Kanbayashi wrote:
>> So, will you give creating the patch a try?
>
> Yes. I try to write the patch for regression test of ecpg command
> warning and error notice :)

Thanks!

> BTW, How should we handle commit fest entry below?
>
> "ecpg command does not warn COPY ... FROM STDIN;"
> https://commitfest.postgresql.org/52/5497/
>
> I think that the patch of regression test is not included the entry.

I've marked this entry as committed.

Please feel free to create a new CF entry for the regression test patch for ecpg.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION


From: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>
To: masao(dot)fujii(at)oss(dot)nttdata(dot)com
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, kuroda(dot)hayato(at)fujitsu(dot)com, tgl(at)sss(dot)pgh(dot)pa(dot)us
Subject: Re: ecpg command does not warn COPY ... FROM STDIN;
Date: 2025-01-17 14:49:46
Message-ID: CANOn0Ex8SaeSTiF3rvZg0kbT3vTJSc15H5n+fUyTZBcAVVTjqQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

🙏

Ryo さんが Gmail
<https://www.google.com/gmail/about/?utm_source=gmail-in-product&utm_medium=et&utm_campaign=emojireactionemail#app>
でリアクションしました

2025年1月17日(金) 午後10:51 Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>:

>
>
> On 2025/01/17 17:46, Ryo Kanbayashi wrote:
> >> So, will you give creating the patch a try?
> >
> > Yes. I try to write the patch for regression test of ecpg command
> > warning and error notice :)
>
> Thanks!
>
> > BTW, How should we handle commit fest entry below?
> >
> > "ecpg command does not warn COPY ... FROM STDIN;"
> > https://commitfest.postgresql.org/52/5497/
> >
> > I think that the patch of regression test is not included the entry.
>
> I've marked this entry as committed.
>
> Please feel free to create a new CF entry for the regression test patch
> for ecpg.
>
> Regards,
>
> --
> Fujii Masao
> Advanced Computing Technology Center
> Research and Development Headquarters
> NTT DATA CORPORATION
>
>