Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
esabol wants to merge3 commits intopostgrespro:master
base:master
Choose a base branch
Loading
fromesabol:add-pg18-to-ci

Conversation

esabol
Copy link
Contributor

@esabolesabol commentedOct 7, 2025•
edited
Loading

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)
  • addsbuffers off toexplain 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)

@esabolesabol marked this pull request as draftOctober 7, 2025 02:08
@esabol
Copy link
ContributorAuthor

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
Copy link
ContributorAuthor

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

@df7cb
Copy link
Contributor

@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
Copy link
ContributorAuthor

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
Copy link
Contributor

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.

esabol reacted with thumbs up emoji

@vitcpp
Copy link
Contributor

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

@esabolesabol marked this pull request as ready for reviewOctober 8, 2025 07:36
@vitcpp
Copy link
Contributor

@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
Copy link
ContributorAuthor

@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
Copy link
Contributor

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

@esabol
Copy link
ContributorAuthor

@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.csrc/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 errorscc1.exe: all warnings being treated as errorsmake: *** [<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....

@esabolesabol changed the titleDraft: Issue #139: Add PostgreSQL 18 builds to CI workflowIssue #139: Add PostgreSQL 18 builds to CI workflow, fix tests on PostgreSQL 18+, and eliminate compilation warningsOct 9, 2025
@esabol
Copy link
ContributorAuthor

esabol commentedOct 9, 2025•
edited
Loading

Well, I figured out how to fix the compilation issues. I addedextern to the yacc/bison variables andstatic to the global variables insrc/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
Copy link
Contributor

Looks good to me!

#defineEULER_AXIS_Z 3/* z - axis for Euler transformation */

externintsphere_yylex();
externintsphere_yychar;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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

@vitcpp
Copy link
Contributor

vitcpp commentedOct 9, 2025•
edited
Loading

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.

image

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

@vitcpp
Copy link
Contributor

vitcpp commentedOct 9, 2025•
edited
Loading

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 reacted with thumbs up emoji

@vitcppvitcpp self-requested a reviewOctober 9, 2025 15:58
@esabol
Copy link
ContributorAuthor

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 ofgcc --version somewhere.

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

Ready to merge at your convenience,@vitcpp.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@vitcppvitcppvitcpp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@esabol@df7cb@vitcpp
[8]ページ先頭
©2009-2025 Movatter.jp