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

Add PostgreSQL database driver #32

Merged
merged 42 commits into from
Mar 11, 2021

Conversation

peace-maker
Copy link
Member

This is regarding bug 3849.
Adds a "pgsql" driver to the dbi system.

See https://forums.alliedmods.net/showthread.php?t=236415

The AMBuild changes are very likely wrong. I don't know how to use it ;)

Here are some instructions on how to create static libs of the libpq out of the source

Get the latest postgresql source.
extract it.

Windows:

In a VS-commandline, navigate to src/tools/msvc and run
perl mkvcbuild.pl
to generate the VS solution in the root.

  1. Open the pgsql.sln in VS2010
  2. change the configuration type of the libpq project from "Dynamic library (.dll)" to "Static library (.lib)"
  3. under C/C++ -> codegeneration change runtimelibrary from "Multithreaded-Debug-DLL (/MDd)" to "Multithreaded-Debug (/MTd)" for the libpq and libpgport projects
  4. compile the libpq project
  5. the static .lib will be in Release/libpq/libpq.lib and Release/libpgport/libpgport.lib

Linux:

in the extracted postgresql source run
./configure LDFLAGS='-static -m32' CFLAGS='-m32' --prefix=pwd/build
You might need to add --without-readline. This is not needed for the libpq library.
cd src/interfaces/libpq
make install
rm libpq.a
make install enable_shared=no
The library will be in the build/lib directory.

About that double linking to get a good static library: http://www.postgresql.org/message-id/1902207738.20070920111734@comodo.com

@peace-maker
Copy link
Member Author

@asherkin pointed out that a simple "make install enable_shared=no" would suffice. That workaround with deleting the lib is only needed if a dynamic library was created before.

I could also just think of a selfhosted package which already includes the static library binaries and checkout-deps.sh would just download that package from alliedmods.net instead of the original source tar from postgresql.org?

@peace-maker
Copy link
Member Author

The latest commit convinces AMBuild to build the static libpq library from the source code archive at postgresql.org. No need to host precompiled binaries anymore which would have been needed to recompile by hand on update. The windows and mac builds will probably need a review when the buildbot actually tries to run.

@peace-maker peace-maker reopened this Jul 25, 2014
cd postgresql-9.3
patch configure.in < ../$sourcemodfolder/extensions/pgsql/configure_autoconf.patch
autoconf
./configure --without-readline
Copy link
Member

Choose a reason for hiding this comment

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

This looks awesome overall, can't wait to get it in the tree for testing.

The only thing is that AMBuild should probably be doing this patching and configuring (so it can work with a manually download source archive). I do much prefer this static linking rather than dropping a non-extension binary in the extensions dir, so it's not a huge deal.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried that, but ambuild doesn't want to copy or move files from directories it doesn't know. If there's some way to just execute |cp|, |patch| and |configure| from within ambuild - shoot!

Copy link
Member

Choose a reason for hiding this comment

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

@peace-maker Something like this should work: https://gist.github.com/dvander/496af643026e1b0cf1d0

This invokes patch with a patch file from inside the source tree, against a file from outside the source tree, placing the output file inside the build folder.

It's important to note two things: (1) AMBuild cannot handle a file that is both an input and output. That means you can't patch in-place. You have to have separate inputs and outputs. (2) When tying commands together, you have to get the inputs right. With patch that's easy. For running configure, you'll need to pass the output of patch as an input. For example,

_, patch_outputs = builder.AddCommand(patch_argv, etc...)
_, autoconf_outputs = builder.AddCommand(
    inputs = patch_outputs + [...anything extra?],
    argv = ['configure'],
    outputs = ['configure'],
)
builder.AddCommand(
    inputs = autoconf_outputs + [... anything extra?],
    ...
)

Copy link
Member

Choose a reason for hiding this comment

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

If you run into problems lemme know and I can take a stab at it. AMBuild2 can be finicky.

@peace-maker
Copy link
Member Author

re: setup steps. As long as they're easy to re-run, no, I don't think it matters. Just note that AMBuild can't be used to build pgsql unless it's in the source tree, so now we're talking about building the static lib as part of checkout-deps...

It actually can build pgsql statically, when doing the configuring and patching in a shell script. That script could be run manually and by the checkout-deps one.
I'll revert the last commit and put the patching of pgsql in a seperate .sh file.

@peace-maker
Copy link
Member Author

Fixing this up to build at least on linux.
I'm not sure with windows buildbots and the "tar" command to unpack the .tar.gz source archive.

Your choice to setup a seperate buildbot to build the pgsql static libs and fetch it in checkout-deps.sh instead of building it again everytime.

@peace-maker
Copy link
Member Author

peace-maker commented Oct 24, 2016

I've rebased this against master.
libpq can be build natively statically on windows now. They don't include the static .lib in their binary distributions though.

You have to make the build compatible with sourcemod's build options first, so change src/interfaces/win32.mak on line 35 from

OPT=/O2 /MD

to

OPT=/O2 /MT

To build on windows it's enough to run the following in the downloaded source

cd src
nmake /f win32.mak

that creates the static lib in interfaces\libpq\Release\libpq.lib
The compilation on linux is covered in the first and second comment.

I'd now vote for a self-hosted archive with the libs and headers.

@spumer
Copy link

spumer commented May 27, 2018

Any plans to merge?

@Andreychik32
Copy link

Looking forward to see this implemented in sourcemod ASAP. When specifying host, user and password in databases.cfg, it is trying to connect to unix socket in /tmp/.s.PGSQL.5432.

@peace-maker
Copy link
Member Author

I've rebased this against master again and fixed not using all of databases.cfg config options.

Copy link
Member

@KyleSanderson KyleSanderson left a comment

Choose a reason for hiding this comment

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

God 5 years of churn... Sorry man - if this still builds (Travis and AppVeyour fail atm) I'll take as-is (please fix the style and nuke the MSVC proj files though) if you can continue to maintain it going forward. I'm not sure any of us can take the maintenance of this on so I'd appreciate if you can continue to review bugs (and for this specifically).

extensions/pgsql/pgsql/PgDriver.cpp Show resolved Hide resolved
extensions/pgsql/pgsql/PgDriver.cpp Outdated Show resolved Hide resolved
extensions/pgsql/pgsql/PgStatement.cpp Outdated Show resolved Hide resolved
extensions/pgsql/pgsql/PgStatement.cpp Outdated Show resolved Hide resolved
Bug #3849
This adds a postgresql database driver called "pgsql".
The ambuild script changes could be very wrong ;)
This was originally made by Lyfe in bug 3849! Thanks for that.
PostgreSQL supports the 'IF NOT EXISTS' clause when creating tables
since version 9.1, so i've switched to use that.
@KyleSanderson
Copy link
Member

@dvander are you okay if we merge this one?

@ErikMinekus
Copy link
Contributor

ErikMinekus commented Aug 23, 2020

I think this needs a prebuilt libpq for Win64, now that Win64 is included in the builds. I haven't been able to build one, so maybe @peace-maker has time to look into it.

Some other things to note:

  • We are using an older libpq version, 9.6.9, as newer versions cannot produce a static library for Windows. We will need to make a custom build script for that.
  • libpq has not been built with SSL support, which some providers require. I believe the MySQL driver doesn't support SSL either though.
  • According to Peace-Maker, libpq does not automatically reconnect when the connection is lost, and there is no indication to the plugin that the connection has been lost, other than an error message when you try to perform a query. Perhaps this has improved in newer PostgreSQL versions, I haven't looked into it.

@drozdowsky

This comment has been minimized.

@KyleSanderson
Copy link
Member

@peace-maker #32 (comment)

Can you please address this (we just went through it with regex, I'm sure it's going to be the same with others). Tag me when it's done please. Please keep in mind we keep MySQL out of tree.

This is version 9.6.15 since that's what I still had laying around.
@peace-maker
Copy link
Member Author

@KyleSanderson Here's the missing win64 libpq build.

@peace-maker
Copy link
Member Author

Lets get this in the tree to poke at further 💯

@peace-maker peace-maker merged commit e5342af into alliedmodders:master Mar 11, 2021
@Alienmario
Copy link
Contributor

Hi
Don't know if you're aware, the driver is unable to connect to more recent psql versions due to this error:

SCRAM authentication requires libpq version 10 or above

I'm guessing that's corresponding to the DB version. I'm on windows.

@psychonic
Copy link
Member

It can connect to those versions without issue, just not with the default authentication type. You are able to change that though.

The issue is being tracked on #1802

@peace-maker peace-maker deleted the postgres branch February 21, 2023 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help Wanted up for grabs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants