Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mingw64 built pg_sphere for PG17 doesn't work under windows VC built PostgreSQL EDB release #130

Open
robe2 opened this issue Oct 14, 2024 · 11 comments
Assignees

Comments

@robe2
Copy link

robe2 commented Oct 14, 2024

I usually build windows binaries under mingw64 and deploy these on PostgreSQL Windows EDB distributions.
This works just fine for the pg_sphere 1.5.2 (master) branch for the 14-16 I have tried building so far, except for PostgreSQL 17.

Note I have already built binaries for PostGIS, MobilityDb, pgRouting, h3 and all those build fine and regress fine.

Everything regresses fine on my mingw64 PG 17 install, but when I try to do

CREATE EXTENSION pg_sphere;

on my PostgreSQL Windows EDB 17, pg_sphere.dll": The specified procedure could not be found.

I've narrowed the issue down to output.c, but I can't figure out what is wrong with that file. Perhaps it's using some function that used to be there for windows for PG < 17 perhaps.

How I know it's the culprit is if I change the Makefile to below - leaving out output.o

OBJS = src/sscan.o src/sparse.o src/sbuffer.o src/vector3d.o src/point.o \
			 src/euler.o src/circle.o src/circle_sel.o src/line.o src/ellipse.o src/polygon.o \
			src/path.o src/box.o src/gq_cache.o src/gist.o src/gist_support.o src/key.o src/gnomo.o src/epochprop.o src/brin.o

I at least get a library that tries to load, but fails on not being able to find the functions in output.c
Any thoughts on what this issue could be?

@esabol
Copy link
Contributor

esabol commented Oct 14, 2024

Our GitHub Actions CI workflow builds and tests with PostgreSQL 17 on Linux. That said, it seems the last build was 4 months ago, well before the final release of PostgreSQL 17.0. So it's possible something changed, but I think it's unlikely.

When reporting any issue, please include the exact error message you're seeing. It's hard for anyone to diagnose the cause otherwise. If we knew the exact functions in output.c, that would be useful.

If I had to guess why you're seeing some issue with Windows not finding some function in output.c, my guess would be that Windows is loading an older pg_sphere.dll. But I'm just pulling that out of a hat without more concrete information.

@robe2
Copy link
Author

robe2 commented Oct 14, 2024

No I don't think it's anything wrong with PG17, cause as I mentioned my mingw64 PG17 works fine and regresses fine.

It's installing the library in windows EDB vc++ builds that fails and just for PG17

I've narrowed down the issue further.

If I remark out all the clauses like

if (sphere_output_precision != INT_MAX)

In the Datum functions of output.c, it installs fine under my PG17 windows VC++ edb build.

which always forces those functions to go thru the out_compat functions.

I just don't understand why this is only in issue with PG17 and not the other versions, I'm assuming maybe it can't resolve SPoint or something for some reason.

FWIW, PG17 is the first version of PostgreSQL to no longer support Visual Studio directly, all those scripts have been ripped out in place of meson. So could be some sort of meson bug involving VC++ that it's not exporting some things.

@robe2
Copy link
Author

robe2 commented Oct 14, 2024

Narrowed down further issue seems to be calling of the inlined function

static inline void
spherepoint_out_buffer(StringInfo si, const SPoint *sp)

@robe2
Copy link
Author

robe2 commented Oct 14, 2024

Narrowed down further issue seems to be calling of the inlined function

static inline void
spherepoint_out_buffer(StringInfo si, const SPoint *sp)

I narrowed the issue down further, it seems to be that eventually calling

static void
pgs_strinfo_put_d64(StringInfo si, double value)

and in the call to

double_to_shortest_decimal_buf(value, buf);

in https://github.com/postgrespro/pgsphere/blob/master/src/output.c#L163

Perhaps the windows vc++ builds aren't exporting that function out in PG17 so might be a bug upstream. I would expect inclusion of #include "common/shortest_dec.h" as you have is sufficient to bring that function in.

@robe2
Copy link
Author

robe2 commented Oct 14, 2024

reported upstream as well - https://www.postgresql.org/message-id/000001db1df8%2449765bb0%24dc631310%24%40pcorp.us

@esabol
Copy link
Contributor

esabol commented Oct 14, 2024

No I don't think it's anything wrong with PG17, cause as I mentioned my mingw64 PG17 works fine and regresses fine.

Um, I didn't say it was?

@vitcpp vitcpp self-assigned this Oct 14, 2024
@vitcpp
Copy link
Contributor

vitcpp commented Oct 22, 2024

Just some thoughts...

pgSphere uses a number of functions from postgresql which are declared with extern specifier. I do not see any linkage problems with such functions. The function double_to_shortest_decimal_buf is declared in the header without extern specifier in REL_17_STABLE. With gcc, by default, it should have external linkage as those functions with extern specifier. Probably, under mswin the situation is different. Some compilation options may differ.

P.S. I'm not sure which postgresql core C functions I can use. If double_to_shortest_decimal_buf is restricted from using in extensions, I should fix it in pgsphere. My opinion, it is important function that should unify the output of doubles in extensions. I would rather keep using of this function.

@esabol
Copy link
Contributor

esabol commented Oct 22, 2024

@vitcpp wrote:

The function double_to_shortest_decimal_buf is declared in the header without extern specifier in REL_17_STABLE.

Was it declared with "extern" in previous versions of PostgreSQL (16.x, 15.x, etc.)?

@vitcpp
Copy link
Contributor

vitcpp commented Oct 23, 2024

@esabol The declaration seems the same since 12.x (without extern), no any differences in shortest_dec.h. I guess the issue is related to some compiler settings.

@vitcpp
Copy link
Contributor

vitcpp commented Oct 23, 2024

I tried to build pg17 (REL_17_STABLE) and pgsphere under ms win using msys2 + mingw64 runtime (pg17 was compiled with some small fix). I used meson to build pg17. pgsphere was compiled without any changes. I succeeded to start pg17 and do create extension pg_sphere. It was completed without any errors.

@robe2 Could you please provide some details about your env? How can I configure the env like yours?

P.S. I forgot to say, I tried the master branch which is not released yet. After fixing this issue, we can create a new 1.5.2 release.

@esabol
Copy link
Contributor

esabol commented Oct 23, 2024

@esabol The declaration seems the same since 12.x (without extern), no any differences in shortest_dec.h. I guess the issue is related to some compiler settings.

Then I don't see why the OP would see different results between compiling the extension with <=16.x and 17.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants