Issue #139: Add PostgreSQL 18 builds to CI workflow, fix tests on PostgreSQL 18+, and eliminate compilation warnings

[esabol]

This merge request is a team effort of @esabol and @vitcpp and makes the following changes:

• adds PostgreSQL 18 builds to the GitHub Action CI workflow (@esabol)
• adds buffers off to explain analyze tests (@vitcpp)
• filters out Index Searches statistics (@vitcpp)
• adds test variants for PostgreSQL 18+ because row statistics is of double type (@vitcpp)
• fixes compilation warnings discovered in PostgreSQL 18 builds on Windows (@esabol)

[esabol]

Hmmm, the CI workflow didn't run upon opening the merge request.

@vitcpp : I think the CI workflows have been disabled. Can you re-enable them, please?

[esabol]

@df7cb : Are you able to enable GitHub Actions on this repo?

[df7cb]

@df7cb : Are you able to enable GitHub Actions on this repo?

I only had admin rights on the "pgsphere" organization, not the postgrespro/pgsphere repo.

[esabol]

Well, I just emailed @vitcpp. I hope he responds. I am concerned because he hasn't had any GitHub activity in almost a year.

[vitcpp]

Hi @esabol, I'm here. I will review the patch and look at other new issues tomorrow. Sorry for the delay. Now it is too late.

[vitcpp]

I enabled actions but not sure how to run them for the PR

[vitcpp]

@esabol Could you please enable and run Build and Test workflow in your fork and branch to make sure everything is ok? I have no idea how to configure workflows in this repo to manually run with the specified PR? We will merge the commit if everything is ok.

[esabol]

@vitcpp : After you merge #141, I will rebase this PR. That should trigger the CI pipeline.

I'm still hoping to fix the broken selectivity test on PostgreSQL 18 as part of this PR. Anyone have a suggestion as to how best to do that?

[vitcpp]

@esabol I'm trying to fix the tests. I can send you a patch when ready.

[esabol]

@vitcpp beat me to it. He sent me a patch to fix the selectivity regression test on PostgreSQL 18. That indeed worked on Linux just fine.

Unfortunately, the Windows build on PostgreSQL 18 isn't working for some reason. It emits the following warnings (which are treated as compilation errors, so it doesn't even run the regression tests):

gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wshadow=compatible-local -Wformat-security -Wmissing-variable-declarations -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -O2 -Werror -Wall -DPGSPHERE_VERSION=1.5.2 -fvisibility=hidden -DPGSPHERE_VERSION=1.5.2 -I. -I./ -ID:/a/_temp/msys64/mingw64/include/postgresql/server -ID:/a/_temp/msys64/mingw64/include/postgresql/internal -D_POSIX_C_SOURCE -I./src/include/port/win32     -ID:/a/_temp/msys64/mingw64/include/postgresql/server/port/win32 -DWIN32_STACK_RLIMIT=4194304  -c -o src/point.o src/point.c
src/sparse.c:74:25: error: no previous declaration for 'sphere_yychar' [-Werror=missing-variable-declarations]
   74 | #define yychar          sphere_yychar
      |                         ^~~~~~~~~~~~~
src/sparse.c:1102:5: note: in expansion of macro 'yychar'
 1102 | int yychar;
      |     ^~~~~~
src/sparse.c:71:25: error: no previous declaration for 'sphere_yynerrs' [-Werror=missing-variable-declarations]
   71 | #define yynerrs         sphere_yynerrs
      |                         ^~~~~~~~~~~~~~
src/sparse.c:1107:5: note: in expansion of macro 'yynerrs'
 1107 | int yynerrs;
      |     ^~~~~~~
src/sbuffer.c:10:15: error: no previous declaration for 'spheretype' [-Werror=missing-variable-declarations]
   10 | unsigned char spheretype;
      |               ^~~~~~~~~~
src/sbuffer.c:[13](https://github.com/postgrespro/pgsphere/actions/runs/18354919577/job/52284337761?pr=140#step:5:14):9: error: no previous declaration for 'bufangle' [-Werror=missing-variable-declarations]
   13 | float8  bufangle[MAX_BUF_ANGLE];
      |         ^~~~~~~~
src/sbuffer.c:27:[17](https://github.com/postgrespro/pgsphere/actions/runs/18354919577/job/52284337761?pr=140#step:5:18): error: no previous declaration for 'bufpoints' [-Werror=missing-variable-declarations]
   27 | }               bufpoints;
      |                 ^~~~~~~~~
src/sbuffer.c:30:17: error: no previous declaration for 'bufline' [-Werror=missing-variable-declarations]
   30 | int             bufline;
      |                 ^~~~~~~
src/sbuffer.c:36:17: error: no previous declaration for 'bufcircle' [-Werror=missing-variable-declarations]
   36 | int             bufcircle[2];
      |                 ^~~~~~~~~
src/sbuffer.c:39:17: error: no previous declaration for 'bufellipse' [-Werror=missing-variable-declarations]
   39 | int             bufellipse[5];
      |                 ^~~~~~~~~~
src/sbuffer.c:42:17: error: no previous declaration for 'bufeuler' [-Werror=missing-variable-declarations]
   42 | int             bufeuler[3];
      |                 ^~~~~~~~
src/sbuffer.c:50:17: error: no previous declaration for 'bufeulertype' [-Werror=missing-variable-declarations]
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wshadow=compatible-local -Wformat-security -Wmissing-variable-declarations -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -O2 -Werror -Wall -DPGSPHERE_VERSION=1.5.2 -fvisibility=hidden -DPGSPHERE_VERSION=1.5.2 -I. -I./ -ID:/a/_temp/msys64/mingw64/include/postgresql/server -ID:/a/_temp/msys64/mingw64/include/postgresql/internal -D_POSIX_C_SOURCE -I./src/include/port/win32     -ID:/a/_temp/msys64/mingw64/include/postgresql/server/port/win32 -DWIN32_STACK_RLIMIT=4[19](https://github.com/postgrespro/pgsphere/actions/runs/18354919577/job/52284337761?pr=140#step:5:20)4304  -c -o src/euler.o src/euler.c
   50 | }               bufeulertype;
      |                 ^~~~~~~~~~~~
src/sbuffer.c:53:17: error: no previous declaration for 'bufapos' [-Werror=missing-variable-declarations]
   53 | int             bufapos;
      |                 ^~~~~~~
src/sbuffer.c:56:17: error: no previous declaration for 'bufspos' [-Werror=missing-variable-declarations]
   56 | int             bufspos;
      |                 ^~~~~~~
src/sbuffer.c:59:9: error: no previous declaration for 'parse_buffer' [-Werror=missing-variable-declarations]
   59 | char   *parse_buffer;
      |         ^~~~~~~~~~~~
cc1.exe: all warnings being treated as errors
cc1.exe: all warnings being treated as errors
make: *** [<builtin>: src/sparse.o] Error 1

I'm confounded as to why this wouldn't be a problem on PostgreSQL 10-17 but is a problem on PostgreSQL 18. Any ideas? I don't really do WIndows....

[esabol]

Well, I figured out how to fix the compilation issues. I added extern to the yacc/bison variables and static to the global variables in src/sbuffer.c. Does that seem like the correct fixes to you, @vitcpp and @df7cb ? All the CI builds passed at least.

Ready to merge if it passes your reviews.

[df7cb]

Looks good to me!

[vitcpp]

I'm not sure about extern declaration of these variables. They are not used in the code except of sparse.c.

[vitcpp]

src/sparse.c:74:25: error: no previous declaration for 'sphere_yychar' [-Werror=missing-variable-declarations]
   74 | #define yychar          sphere_yychar

The problem here, I guess, the sphere_yychar is a generated name (sphere_yy + char). It should substitute yychar use in the sparse.c generated code. It seems, gcc under windows thinks that it is variable and it should be declared somewhere. It looks like a false error. Extern helps to fix this wrong warning but it externs this symbol which is not a good thing. I propose to think how to improve it.

It seems to happen on gcc 14+. Below is the screenshot from godbolt.org where this problem is reproduced. This error was not reproduced with gcc 13 and earlier, because missing-variable-declarations option is not implemented, the check seems to absent.

I think, pgsphere will not compile with gcc 14+ with this option enabled as error

[vitcpp]

I looked deeply into this stuff and found that the sphere_yychar, sphere_yynerrs are the global variables which should be declared with extern in the generated parse.h header. Once, yacc doesn't declare it in the header, the fix seems to me ok as a workaround. I propose to merge it.

[esabol]

It seems to happen on gcc 14+. Below is the screenshot from godbolt.org where this problem is reproduced. This error was not reproduced with gcc 13 and earlier, because missing-variable-declarations option is not implemented, the check seems to absent.

Yes. I had a comment that said as much, but I neglected to post it. What I don't understand is why the PostgresSQL 18 Windows build is using gcc 14+ and the PostgreSQL 10-17 builds are apparently using an older version of gcc.

It would be nice if the GitHub Actions CI workflows displayed the output of gcc --version somewhere.

I think marking these yacc/bison variables as extern in some header is correct. I imagine newer versions of bison or yacc would autogenerate .c/.h files with extern already in them, but I don't know.

Ready to merge at your convenience, @vitcpp.