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

Postgres version string from ubuntu still causing errors [BugFix] #105

Closed
guionardo opened this issue Oct 20, 2020 · 7 comments
Closed

Postgres version string from ubuntu still causing errors [BugFix] #105

guionardo opened this issue Oct 20, 2020 · 7 comments

Comments

@guionardo
Copy link

When the PosgreSQL version has values like this: "12.4 (Debian 12.4-1.pgdg100+1)", a exception is raised in versionstring.split.
I added some normalization code to avoid this.

I don't have permission to contribute here with PR. So this is the patch to fix this.

def assure_version(v:str)->str:
	'''Assure the version from postgres is in 99.99[.99] format'''
	import re

	for match in re.finditer("(\d{1,2}.\d{1,2})(\.\d{1,2})?", v):
		return match.group()
	return v

Change the line 19 of postgresql/versionstring.py
from

	v = vstr.strip().split('.')

to

	v = assure_version(vstr).strip().split('.')
@jwp
Copy link
Contributor

jwp commented Oct 21, 2020

Your efforts are appreciated, however, I believe debian offers a py-postgresql package handling their special case.

Supporting such variations in the version string and the protocol is not within the scope of the project. If such compensation is desired, you will, unfortunately, need to manage a patched distribution yourself. (A runtime patch is also possible)

You may also want to consider using: https://www.postgresql.org/download/linux/debian/
I suspect apt.postgresql.org has managed to avoid modifying the version.

Future versions (py-postgresql) may make this a non-issue. Presumably, we can drop support for 8.5 and defer version parsing, but applications needing to switch on server versions would still need a patched variant to handle debian's case.

@jwp jwp closed this as completed Oct 21, 2020
@EdTorbett
Copy link

I've just encountered this same issue and don't see the above as a suitable resolution to this issue. Firstly, I am connecting to the database remotely and from a different operating system, so cannot rely on an OS-specific package release to resolve the issue. Secondly, no other client library gives the same error when connecting to the same database. If the above patch resolves this issue, why reject it?

@jwp
Copy link
Contributor

jwp commented Nov 1, 2020

In the short term, I think the runtime patch is something like:

import postgresql.versionstring as vs
def parse_debian_compat(vstr, _split=vs.split):
    return _split(vstr.split()[0])
vs.split = parse_debian_compat

Didn't test, so you might need to adjust it. Be sure to run that before doing other postgresql.* imports.

@jwp
Copy link
Contributor

jwp commented Nov 2, 2020

In the long term, I would advise not using a postgresql package that modifies that particular version string.
I suspect apt.postgresql.org will work. https://www.postgresql.org/download/linux/debian/

While other drivers connect successfully, applications that choose to switch on the version may be broken as well.
py-postgresql is revealing a symptom. The modification of that particular version string only serves to add risk.

On the py-postgresql side, the best that can be done is an exception trap defaulting the version.
Handling debian's (or other's) special case is simply out of scope. The only reason that this is even considerable is due to the possibility that a PostgreSQL fork or PQ server could, reasonably, make the mistake of omitting or adjusting this string substantially.

However, this driver is only tested against builds of source releases made by postgresql.org. If you're using it to connect to something that is not PostgreSQL, you will need to test it and likely make modifications to compensate.

@jwp jwp mentioned this issue Nov 11, 2020
@c7hm4r
Copy link

c7hm4r commented Mar 10, 2021

@jwp Why do you think that it is part of PostgreSQL’s specification that there must not be spaces in the preset option server_version? I could not find that this assumption is being made in the PostgreSQL project, neither officially (docs1, docs2) nor internally (source code). The source code of PostgreSQL seems to only assume that the version string starts with an integer. Or is your goal to just support a subset of PostgreSQL?

@jwp
Copy link
Contributor

jwp commented Mar 11, 2021

I'm not interested in debating this further. I have been quite clear about my position. If you find this unacceptable, I recommend that you take your own advice and use another driver (#113).

@c7hm4r
Copy link

c7hm4r commented Mar 11, 2021 via email

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

4 participants