Horton

Thread

Commits

  1. ecpg: Fix compiler warning in ecpg build with Meson.

  2. ecpg: Add TAP test for the ecpg command.

  3. Avoid updating inactive_since for invalid replication slots.

  1. [PATCH] Add regression tests of ecpg command notice (error / warning)

    Ryo Kanbayashi <kanbayashi.dev@gmail.com> — 2025-02-05T12:31:28Z

    Hi hackers,

When I wrote patch of ecpg command notice bug, I recognized needs of
regression tests for ecpg command notices and I say that I write the
tests.

https://commitfest.postgresql.org/52/5497/
https://www.postgresql.org/message-id/0efab1f6-5d8d-451f-a7dc-ef9c73ba9e02%40oss.nttdata.com

This mail is about patch of the tests.

Patch is attached to this mail : ecpg_cmd_notice_regress_test.patch
This patch passed CI.
https://cirrus-ci.com/build/4861827500212224

And corresponding diff is below.
https://github.com/ryogrid/postgres/compare/0ec3c295e7594ed3af86bca1a4b4be269c2f069d...72329311a75630594bcaa38248255360b7e8e525

I explain about implementation of this patch.

What is this patch
- add regression tests which test ecpg command notices such as warning
and errors
- test notices implemented in ecpg.addons file

Basic policy on implementation
- do in a way that matches the method using the existing pg_regress
command as much as possible
- avoid methods that increase the scope of influence

Next, I list answers to points that are likely to be pointed out in
advance below :)
- shell scripts and bat files is used due to ...
    avoid non zero exit code of ecpg command makes tests failure
    avoid increasing C code for executing binary which cares cross platform
- python code is used because I couldn't write meson.build
appropriately describe dependency about materials which is used on
tests without it. please help me...
- as you said, kick this kind of tests by pg_regress accompanied with
needless PG server process execution. but pg_regress doesn't execute
test without it and making pg_regress require modification which has
not small scope of influence

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

  2. Re: [PATCH] Add regression tests of ecpg command notice (error / warning)

    Ryo Kanbayashi <kanbayashi.dev@gmail.com> — 2025-02-05T23:57:18Z

    On Wed, Feb 5, 2025 at 9:31 PM Ryo Kanbayashi <kanbayashi.dev@gmail.com> wrote:
>
> Hi hackers,
>
> When I wrote patch of ecpg command notice bug, I recognized needs of
> regression tests for ecpg command notices and I say that I write the
> tests.
>
> https://commitfest.postgresql.org/52/5497/
> https://www.postgresql.org/message-id/0efab1f6-5d8d-451f-a7dc-ef9c73ba9e02%40oss.nttdata.com
>
> This mail is about patch of the tests.
>
> Patch is attached to this mail : ecpg_cmd_notice_regress_test.patch
> This patch passed CI.
> https://cirrus-ci.com/build/4861827500212224
>
> And corresponding diff is below.
> https://github.com/ryogrid/postgres/compare/0ec3c295e7594ed3af86bca1a4b4be269c2f069d...72329311a75630594bcaa38248255360b7e8e525
>
> I explain about implementation of this patch.
>
> What is this patch
> - add regression tests which test ecpg command notices such as warning
> and errors
> - test notices implemented in ecpg.addons file
>
> Basic policy on implementation
> - do in a way that matches the method using the existing pg_regress
> command as much as possible
> - avoid methods that increase the scope of influence
>
> Next, I list answers to points that are likely to be pointed out in
> advance below :)
> - shell scripts and bat files is used due to ...
>     avoid non zero exit code of ecpg command makes tests failure
>     avoid increasing C code for executing binary which cares cross platform
> - python code is used because I couldn't write meson.build
> appropriately describe dependency about materials which is used on
> tests without it. please help me...
> - as you said, kick this kind of tests by pg_regress accompanied with
> needless PG server process execution. but pg_regress doesn't execute
> test without it and making pg_regress require modification which has
> not small scope of influence

Sorry, I re-send patch because a patch already sent includes needless
stderr output file.
NEW PATCH FILENAME: ecpg_cmd_notice_regress_test2.patch

In addition, diff between patches does not affect test behavior.

---
Sincerely,
Ryo Kanbayashi
https://github.com/ryogrid

  3. Fwd: [PATCH] Add regression tests of ecpg command notice (error / warning)

    Ryo Kanbayashi <kanbayashi.dev@gmail.com> — 2025-02-05T23:58:46Z

    On Wed, Feb 5, 2025 at 9:31 PM Ryo Kanbayashi <kanbayashi.dev@gmail.com> wrote:
>
> Hi hackers,
>
> When I wrote patch of ecpg command notice bug, I recognized needs of
> regression tests for ecpg command notices and I say that I write the
> tests.
>
> https://commitfest.postgresql.org/52/5497/
> https://www.postgresql.org/message-id/0efab1f6-5d8d-451f-a7dc-ef9c73ba9e02%40oss.nttdata.com
>
> This mail is about patch of the tests.
>
> Patch is attached to this mail : ecpg_cmd_notice_regress_test.patch
> This patch passed CI.
> https://cirrus-ci.com/build/4861827500212224
>
> And corresponding diff is below.
> https://github.com/ryogrid/postgres/compare/0ec3c295e7594ed3af86bca1a4b4be269c2f069d...72329311a75630594bcaa38248255360b7e8e525
>
> I explain about implementation of this patch.
>
> What is this patch
> - add regression tests which test ecpg command notices such as warning
> and errors
> - test notices implemented in ecpg.addons file
>
> Basic policy on implementation
> - do in a way that matches the method using the existing pg_regress
> command as much as possible
> - avoid methods that increase the scope of influence
>
> Next, I list answers to points that are likely to be pointed out in
> advance below :)
> - shell scripts and bat files is used due to ...
>     avoid non zero exit code of ecpg command makes tests failure
>     avoid increasing C code for executing binary which cares cross platform
> - python code is used because I couldn't write meson.build
> appropriately describe dependency about materials which is used on
> tests without it. please help me...
> - as you said, kick this kind of tests by pg_regress accompanied with
> needless PG server process execution. but pg_regress doesn't execute
> test without it and making pg_regress require modification which has
> not small scope of influence

Sorry, I re-send patch because a patch already sent includes needless
stderr output file.
NEW PATCH FILENAME: ecpg_cmd_notice_regress_test2.patch

In addition, diff between patches does not affect test behavior.

---
Sincerely,
Ryo Kanbayashi
https://github.com/ryogrid

  4. Re: [PATCH] Add regression tests of ecpg command notice (error / warning)

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-02-13T13:49:06Z

    
On 2025/02/06 8:57, Ryo Kanbayashi wrote:
> On Wed, Feb 5, 2025 at 9:31 PM Ryo Kanbayashi <kanbayashi.dev@gmail.com> wrote:
>>
>> Hi hackers,
>>
>> When I wrote patch of ecpg command notice bug, I recognized needs of
>> regression tests for ecpg command notices and I say that I write the
>> tests.

Thanks for working on this!


>> I explain about implementation of this patch.
>>
>> What is this patch
>> - add regression tests which test ecpg command notices such as warning
>> and errors
>> - test notices implemented in ecpg.addons file
>>
>> Basic policy on implementation
>> - do in a way that matches the method using the existing pg_regress
>> command as much as possible
>> - avoid methods that increase the scope of influence
>>
>> Next, I list answers to points that are likely to be pointed out in
>> advance below :)
>> - shell scripts and bat files is used due to ...
>>      avoid non zero exit code of ecpg command makes tests failure
>>      avoid increasing C code for executing binary which cares cross platform
>> - python code is used because I couldn't write meson.build
>> appropriately describe dependency about materials which is used on
>> tests without it. please help me...
>> - as you said, kick this kind of tests by pg_regress accompanied with
>> needless PG server process execution. but pg_regress doesn't execute
>> test without it and making pg_regress require modification which has
>> not small scope of influence

Wouldn't it be simpler to use the existing TAP test mechanism,
as shown in the attached patch? Please note that this patch is very WIP,
so there would be many places that need further implementation and refinement.

Regards,

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

  5. Re: [PATCH] Add regression tests of ecpg command notice (error / warning)

    Ryo Kanbayashi <kanbayashi.dev@gmail.com> — 2025-02-18T03:49:47Z

    On Thu, Feb 13, 2025 at 10:49 PM Fujii Masao
<masao.fujii@oss.nttdata.com> wrote:
>
>
>
> On 2025/02/06 8:57, Ryo Kanbayashi wrote:
> > On Wed, Feb 5, 2025 at 9:31 PM Ryo Kanbayashi <kanbayashi.dev@gmail.com> wrote:
> >>
> >> Hi hackers,
> >>
> >> When I wrote patch of ecpg command notice bug, I recognized needs of
> >> regression tests for ecpg command notices and I say that I write the
> >> tests.
>
> Thanks for working on this!
>
>
> >> I explain about implementation of this patch.
> >>
> >> What is this patch
> >> - add regression tests which test ecpg command notices such as warning
> >> and errors
> >> - test notices implemented in ecpg.addons file
> >>
> >> Basic policy on implementation
> >> - do in a way that matches the method using the existing pg_regress
> >> command as much as possible
> >> - avoid methods that increase the scope of influence
> >>
> >> Next, I list answers to points that are likely to be pointed out in
> >> advance below :)
> >> - shell scripts and bat files is used due to ...
> >>      avoid non zero exit code of ecpg command makes tests failure
> >>      avoid increasing C code for executing binary which cares cross platform
> >> - python code is used because I couldn't write meson.build
> >> appropriately describe dependency about materials which is used on
> >> tests without it. please help me...
> >> - as you said, kick this kind of tests by pg_regress accompanied with
> >> needless PG server process execution. but pg_regress doesn't execute
> >> test without it and making pg_regress require modification which has
> >> not small scope of influence
>
> Wouldn't it be simpler to use the existing TAP test mechanism,
> as shown in the attached patch? Please note that this patch is very WIP,
> so there would be many places that need further implementation and refinement.

Fujii San,

Thank you for reviewing and indication of better implementation.
I rewrite my patch based on your reference implementation :)

---
Great regards,
Ryo Kanbayashi
NTT Open Source Software Center

  6. Re: [PATCH] Add regression tests of ecpg command notice (error / warning)

    Ryo Kanbayashi <kanbayashi.dev@gmail.com> — 2025-02-28T00:24:10Z

    On Tue, Feb 18, 2025 at 12:49 PM Ryo Kanbayashi
<kanbayashi.dev@gmail.com> wrote:
>
> On Thu, Feb 13, 2025 at 10:49 PM Fujii Masao
> <masao.fujii@oss.nttdata.com> wrote:
> >
> >
> >
> > On 2025/02/06 8:57, Ryo Kanbayashi wrote:
> > > On Wed, Feb 5, 2025 at 9:31 PM Ryo Kanbayashi <kanbayashi.dev@gmail.com> wrote:
> > >>
> > >> Hi hackers,
> > >>
> > >> When I wrote patch of ecpg command notice bug, I recognized needs of
> > >> regression tests for ecpg command notices and I say that I write the
> > >> tests.
> >
> > Thanks for working on this!
> >
> >
> > >> I explain about implementation of this patch.
> > >>
> > >> What is this patch
> > >> - add regression tests which test ecpg command notices such as warning
> > >> and errors
> > >> - test notices implemented in ecpg.addons file
> > >>
> > >> Basic policy on implementation
> > >> - do in a way that matches the method using the existing pg_regress
> > >> command as much as possible
> > >> - avoid methods that increase the scope of influence
> > >>
> > >> Next, I list answers to points that are likely to be pointed out in
> > >> advance below :)
> > >> - shell scripts and bat files is used due to ...
> > >>      avoid non zero exit code of ecpg command makes tests failure
> > >>      avoid increasing C code for executing binary which cares cross platform
> > >> - python code is used because I couldn't write meson.build
> > >> appropriately describe dependency about materials which is used on
> > >> tests without it. please help me...
> > >> - as you said, kick this kind of tests by pg_regress accompanied with
> > >> needless PG server process execution. but pg_regress doesn't execute
> > >> test without it and making pg_regress require modification which has
> > >> not small scope of influence
> >
> > Wouldn't it be simpler to use the existing TAP test mechanism,
> > as shown in the attached patch? Please note that this patch is very WIP,
> > so there would be many places that need further implementation and refinement.
>
> Fujii San,
>
> Thank you for reviewing and indication of better implementation.
> I rewrite my patch based on your reference implementation :)

Fujii San and other hackers,

I have rewrote my patch on TAP test sttyle :)
File for build are also updated.

Commitfest entry:
https://commitfest.postgresql.org/patch/5543/

---
Great regards,
Ryo Kanbayashi
NTT Open Source Software Center

  7. Re: [PATCH] Add regression tests of ecpg command notice (error / warning)

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-02-28T14:27:09Z

    
On 2025/02/28 9:24, Ryo Kanbayashi wrote:
> I have rewrote my patch on TAP test sttyle :)
> File for build are also updated.

Thanks for updating the patch!

+    'tests': [
+      't/001_ecpg_notice.pl',
+      't/002_ecpg_notice_informix.pl',

Since neither test emits "notice" messages, shouldn't the test file
names be revised to reflect this?

Also, I'm unsure if it's ideal to place input files directly under
the "t" directory. I looked for similar TAP tests with input files,
but I couldn't find any examples to guide this decision...

+program_help_ok('ecpg');
+program_version_ok('ecpg');
+program_options_handling_ok('ecpg');
+command_fails(['ecpg'], 'ecpg without arguments fails');

These checks seem unnecessary in 002 since they're already covered in 001.

Regards,

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

  8. Re: [PATCH] Add regression tests of ecpg command notice (error / warning)

    Ryo Kanbayashi <kanbayashi.dev@gmail.com> — 2025-03-01T10:45:15Z

    On Fri, Feb 28, 2025 at 11:27 PM Fujii Masao
<masao.fujii@oss.nttdata.com> wrote:
> On 2025/02/28 9:24, Ryo Kanbayashi wrote:
> > I have rewrote my patch on TAP test sttyle :)
> > File for build are also updated.
>
> Thanks for updating the patch!

Thanks for review:)

>
> +    'tests': [
> +      't/001_ecpg_notice.pl',
> +      't/002_ecpg_notice_informix.pl',
>
> Since neither test emits "notice" messages, shouldn't the test file
> names be revised to reflect this?

I replaced "notice" to "err_warn_msg"

> Also, I'm unsure if it's ideal to place input files directly under
> the "t" directory. I looked for similar TAP tests with input files,
> but I couldn't find any examples to guide this decision...

I couldn't too. So places are not changed.

> +program_help_ok('ecpg');
> +program_version_ok('ecpg');
> +program_options_handling_ok('ecpg');
> +command_fails(['ecpg'], 'ecpg without arguments fails');
>
> These checks seem unnecessary in 002 since they're already covered in 001.

I reflected above.

---
Great regards,
Ryo Kanbayashi

  9. Re: [PATCH] Add regression tests of ecpg command notice (error / warning)

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-03-03T03:23:56Z

    
On 2025/03/01 19:45, Ryo Kanbayashi wrote:
>> +program_help_ok('ecpg');
>> +program_version_ok('ecpg');
>> +program_options_handling_ok('ecpg');
>> +command_fails(['ecpg'], 'ecpg without arguments fails');
>>
>> These checks seem unnecessary in 002 since they're already covered in 001.
> 
> I reflected above.

Thanks for updating the patch!

I've made some minor fixes and cosmetic adjustments.
The updated patch is attached.

Unless there are any objections, I'll commit it.

Regards,

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

  10. Re: [PATCH] Add regression tests of ecpg command notice (error / warning)

    Ryo Kanbayashi <kanbayashi.dev@gmail.com> — 2025-03-03T05:09:45Z

    >On Mon, Mar 3, 2025 at 12:23 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
> On 2025/03/01 19:45, Ryo Kanbayashi wrote:
> >> +program_help_ok('ecpg');
> >> +program_version_ok('ecpg');
> >> +program_options_handling_ok('ecpg');
> >> +command_fails(['ecpg'], 'ecpg without arguments fails');
> >>
> >> These checks seem unnecessary in 002 since they're already covered in 001.
> >
> > I reflected above.
>
> Thanks for updating the patch!
>
> I've made some minor fixes and cosmetic adjustments.
> The updated patch is attached.
>
> Unless there are any objections, I'll commit it.

Thanks for reviewing and adustments.
There is no objections :)

---
Great regards,
Ryo Kanbayashi
NTT Open Source Software Center

  11. Re: [PATCH] Add regression tests of ecpg command notice (error / warning)

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-03-04T06:01:56Z

    
On 2025/03/03 14:09, Ryo Kanbayashi wrote:
>> On Mon, Mar 3, 2025 at 12:23 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>> On 2025/03/01 19:45, Ryo Kanbayashi wrote:
>>>> +program_help_ok('ecpg');
>>>> +program_version_ok('ecpg');
>>>> +program_options_handling_ok('ecpg');
>>>> +command_fails(['ecpg'], 'ecpg without arguments fails');
>>>>
>>>> These checks seem unnecessary in 002 since they're already covered in 001.
>>>
>>> I reflected above.
>>
>> Thanks for updating the patch!
>>
>> I've made some minor fixes and cosmetic adjustments.
>> The updated patch is attached.
>>
>> Unless there are any objections, I'll commit it.
> 
> Thanks for reviewing and adustments.
> There is no objections :)

I've pushed the patch. Thanks!

Regards,

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

  12. Re: [PATCH] Add regression tests of ecpg command notice (error / warning)

    Jacob Champion <jacob.champion@enterprisedb.com> — 2025-03-04T22:26:47Z

    On Mon, Mar 3, 2025 at 10:02 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
> I've pushed the patch. Thanks!

Hi all,

> +tests += {
> +  'name': 'ecpg',
> +  'sd': meson.current_source_dir(),
> +  'bd': meson.current_build_dir(),
> +  'tap': {
> +    'tests': [
> +      't/001_ecpg_err_warn_msg.pl',
> +      't/002_ecpg_err_warn_msg_informix.pl',
> +    ],
> +    'deps': ecpg_exe,
> +  },
> +}

My version of Meson is complaining about this use of 'deps':

    ../meson.build:3603: WARNING: Project targets '>=0.54' but uses
feature introduced in '0.60.0': list.<plus>. The right hand operand
was not a list.
    Adding test "ecpg/001_ecpg_err_warn_msg"
    ...

ecpg_exe should perhaps be wrapped in a list for now? I.e.

    -    'deps': ecpg_exe,
    +    'deps': [ecpg_exe],

Thanks,
--Jacob

  13. Re: [PATCH] Add regression tests of ecpg command notice (error / warning)

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-03-05T00:32:00Z

    
On 2025/03/05 7:26, Jacob Champion wrote:
> On Mon, Mar 3, 2025 at 10:02 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>> I've pushed the patch. Thanks!
> 
> Hi all,
> 
>> +tests += {
>> +  'name': 'ecpg',
>> +  'sd': meson.current_source_dir(),
>> +  'bd': meson.current_build_dir(),
>> +  'tap': {
>> +    'tests': [
>> +      't/001_ecpg_err_warn_msg.pl',
>> +      't/002_ecpg_err_warn_msg_informix.pl',
>> +    ],
>> +    'deps': ecpg_exe,
>> +  },
>> +}
> 
> My version of Meson is complaining about this use of 'deps':
> 
>      ../meson.build:3603: WARNING: Project targets '>=0.54' but uses
> feature introduced in '0.60.0': list.<plus>. The right hand operand
> was not a list.
>      Adding test "ecpg/001_ecpg_err_warn_msg"
>      ...
> 
> ecpg_exe should perhaps be wrapped in a list for now? I.e.
> 
>      -    'deps': ecpg_exe,
>      +    'deps': [ecpg_exe],

Thanks for reporting this and suggesting a fix. I think you're right.

I confirmed that the compiler warning also appears in my environment,
and your fix resolves it. I’ve converted your fix into a patch, which is attached.

Unless there are any objections, I'm thinking to commit it.

Regards,

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

  14. Re: [PATCH] Add regression tests of ecpg command notice (error / warning)

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-03-05T23:25:39Z

    
On 2025/03/05 9:32, Fujii Masao wrote:
> 
> 
> On 2025/03/05 7:26, Jacob Champion wrote:
>> On Mon, Mar 3, 2025 at 10:02 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>>> I've pushed the patch. Thanks!
>>
>> Hi all,
>>
>>> +tests += {
>>> +  'name': 'ecpg',
>>> +  'sd': meson.current_source_dir(),
>>> +  'bd': meson.current_build_dir(),
>>> +  'tap': {
>>> +    'tests': [
>>> +      't/001_ecpg_err_warn_msg.pl',
>>> +      't/002_ecpg_err_warn_msg_informix.pl',
>>> +    ],
>>> +    'deps': ecpg_exe,
>>> +  },
>>> +}
>>
>> My version of Meson is complaining about this use of 'deps':
>>
>>      ../meson.build:3603: WARNING: Project targets '>=0.54' but uses
>> feature introduced in '0.60.0': list.<plus>. The right hand operand
>> was not a list.
>>      Adding test "ecpg/001_ecpg_err_warn_msg"
>>      ...
>>
>> ecpg_exe should perhaps be wrapped in a list for now? I.e.
>>
>>      -    'deps': ecpg_exe,
>>      +    'deps': [ecpg_exe],
> 
> Thanks for reporting this and suggesting a fix. I think you're right.
> 
> I confirmed that the compiler warning also appears in my environment,
> and your fix resolves it. I’ve converted your fix into a patch, which is attached.
> 
> Unless there are any objections, I'm thinking to commit it.

I've pushed the patch. Thanks!

Regards,

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