Postgres Pro
Downloads
Home   >   mailing lists

Thread: Re: Fix an error while building test_radixtree.c with TEST_SHARED_RT

Re: Fix an error while building test_radixtree.c with TEST_SHARED_RT

From
Heikki Linnakangas
Date:
On 19/11/2024 01:20, Masahiko Sawada wrote:
> I realized that building test_radixtree.c with TEST_SHARED_RT fails
> because it eventually sets RT_SHMEM when #include'ing radixtree.h but
> it's missing some header files to include. I've attached a patch to
> include necessary header files in radixtree.h to make it
> self-contained.

+1. Please make sure the #includes are in alphabetical order.

While we're at it, I noticed that lib/radixtree.h includes "postgres.h". 
That's against our usual convention.

-- 
Heikki Linnakangas
Neon (https://neon.tech)

Re: Fix an error while building test_radixtree.c with TEST_SHARED_RT

From
Tom Lane
Date:
Masahiko Sawada <sawada.mshk@gmail.com> writes:
> On Mon, Nov 18, 2024 at 3:41 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> While we're at it, I noticed that lib/radixtree.h includes "postgres.h".
>> That's against our usual convention.

> Good catch. I've updated the patch accordingly.

Probably out of scope for this particular patch, but it occurred to me
to grep for other similar violations, and I found three:

src/bin/pg_combinebackup/copy_file.h:#include "c.h"
src/include/fe_utils/option_utils.h:#include "postgres_fe.h"
src/include/fe_utils/query_utils.h:#include "postgres_fe.h"

            regards, tom lane

Re: Fix an error while building test_radixtree.c with TEST_SHARED_RT

From
Masahiko Sawada
Date:
On Mon, Nov 18, 2024 at 4:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Masahiko Sawada <sawada.mshk@gmail.com> writes:
> > On Mon, Nov 18, 2024 at 3:41 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> >> While we're at it, I noticed that lib/radixtree.h includes "postgres.h".
> >> That's against our usual convention.
>
> > Good catch. I've updated the patch accordingly.
>
> Probably out of scope for this particular patch, but it occurred to me
> to grep for other similar violations, and I found three:
>
> src/bin/pg_combinebackup/copy_file.h:#include "c.h"
> src/include/fe_utils/option_utils.h:#include "postgres_fe.h"
> src/include/fe_utils/query_utils.h:#include "postgres_fe.h"

Good catch, make sense to fix them too. Probably we can fix them
including the removal of inclusion of postgres.h from radixtree.h in
one commit, and fix the inclusion issue in radixtree.h in another
commit.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com