Re: pgsql: Implement pipeline mode in libpq

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, craig(dot)ringer(at)enterprisedb(dot)com, matthieu(dot)garrigues(at)gmail(dot)com
Cc: pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: Re: pgsql: Implement pipeline mode in libpq
Date: 2021-04-06 10:43:17
Message-ID: CAApHDvrw=HHESn1UR9MYuHuqMMbJRk6VFHFwioq0P+xO-D5QXA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

On Tue, 30 Mar 2021 at 20:19, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> On Tue, 16 Mar 2021 at 10:20, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> >
> > Implement pipeline mode in libpq
>
> > src/test/modules/libpq_pipeline/libpq_pipeline.c | 1303 ++++++++++++++++++++
>
> I'm wondering if you meant to leave the "#define DEBUG" line at line
> 34 in the above file?
>
> It seems pretty strange to do:
>
> #define DEBUG
> #ifdef DEBUG
> #define pg_debug(...) do { fprintf(stderr, __VA_ARGS__); } while (0)
> #else
> #define pg_debug(...)
> #endif
>
> pg_debug will never be an empty macro.
>
> I noticed this when testing compiling postgres with MSVC. The compiler
> is kicking out a warning: 'DEBUG': macro redefinition, which will be
> because that compiler defines DEBUG when doing non-production builds.

I'm still not really sure why this code does #define directly followed
by an #ifdef for that define. However, just to clear up the final
compiler warning that's currently being produced on MSVC, I've
attached a proposed patch to fix it.

None of the buildfarm animals are producing this warning as they all
seem to be building as "Release" builds.

libpq_pipeline.c on my machine is compiled with the following:

C:\Program Files (x86)\Microsoft Visual
Studio\2017\Community\VC\Tools\MSVC\14.16.27023\bin\HostX86\x64\CL.exe
/c /Isrc/include /Isrc/include/port/win32
/Isrc/include/port/win32_msvc /Isrc\interfaces\libpq /Zi /nologo /W3
/WX- /diagnostics:classic /Od /D WIN32 /D _WINDOWS /D __WINDOWS__ /D
__WIN32__ /D WIN32_STACK_RLIMIT=4194304 /D _CRT_SECURE_NO_DEPRECATE /D
_CRT_NONSTDC_NO_DEPRECATE /D _DEBUG /D DEBUG=1_MBCS /GF- /Gm- /EHsc
/MDd /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline
/Fo".\Debug\libpq_pipeline\\" /Fd".\Debug\libpq_pipeline\vc141.pdb"
/Gd /TC /wd4018 /wd4244 /wd4273 /wd4102 /wd4090 /wd4267 /FC
/errorReport:queue /MP
src/test/modules/libpq_pipeline/libpq_pipeline.c
libpq_pipeline.c

Note: /D DEBUG=1_MBCS

This causes:

Build succeeded.

"L:\Projects\Postgres\e\pgsql.sln" (default target) (1) ->
"L:\Projects\Postgres\e\libpq_pipeline.vcxproj" (default target) (88) ->
(ClCompile target) ->
l:\projects\postgres\e\src\test\modules\libpq_pipeline\libpq_pipeline.c(38):
warning C4005: 'DEBUG': macro redefinition
[L:\Projects\Postgres\e\libpq_pipeline.vcxproj]

1 Warning(s)
0 Error(s)

Whereas on hamerkop, they are [1]:

C:\\Program Files (x86)\\Microsoft Visual
Studio\\2017\\Community\\VC\\Tools\\MSVC\\14.15.26726\\bin\\HostX86\\x64\\CL.exe
/c /Isrc/include /Isrc/include/port/win32
/Isrc/include/port/win32_msvc /I"c:\\pkg\\zlib-dist\\include"
/I"c:\\OpenSSL-Win64\\include" /Isrc\\interfaces\\libpq /Zi /nologo
/W3 /WX- /diagnostics:classic /Ox /D WIN32 /D _WINDOWS /D __WINDOWS__
/D __WIN32__ /D WIN32_STACK_RLIMIT=4194304 /D _CRT_SECURE_NO_DEPRECATE
/D _CRT_NONSTDC_NO_DEPRECATE /D _MBCS /GF /Gm- /EHsc /MD /GS
/fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline
/Fo".\\Release\\libpq_pipeline\\\\"
/Fd".\\Release\\libpq_pipeline\\vc141.pdb" /Gd /TC /wd4018 /wd4244
/wd4273 /wd4102 /wd4090 /wd4267 /FC /errorReport:queue /MP
src/test/modules/libpq_pipeline/libpq_pipeline.c
libpq_pipeline.c

Note the mention of "Release", indicating the Release rather than Debug build.

Does anyone want to comment on the #define just before the #ifdef?

David

[1] https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=hamerkop&dt=2021-04-02%2010%3A18%3A13&stg=make

Attachment Content-Type Size
fix_compiler_warning_on_msvc_debug_builds.patch application/octet-stream 558 bytes

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Dean Rasheed 2021-04-06 10:52:28 pgsql: pgbench: Function to generate random permutations.
Previous Message Etsuro Fujita 2021-04-06 10:15:41 pgsql: Adjust input value to WaitEventSetWait() in ExecAppendAsyncEvent