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 support for PostgreSQL with PyGreSQL #234

Closed
wants to merge 3 commits into from
Closed

Conversation

Cito
Copy link
Contributor

@Cito Cito commented Feb 6, 2016

a description

@zzzeek
Copy link
Owner

zzzeek commented Feb 6, 2016

looks good, how well do the tests run ? can you paste an output

The dialect may be initialized without the dbapi module being loaded.
Also, a connection may be made with no host argument set (localhost).
@Cito
Copy link
Contributor Author

Cito commented Feb 7, 2016

Thanks for the quick feedback.

Did some manual testing and ran the standard test suite. Just noticed that the standard test suite only tests the default subdialect set as base.dialect (psycopg2 in this case). So I changed the base.dialect temporarily to pygresql, and ran the test suite again (is there a better was of testing this?). After fixing some issues (see above), the test suite now runs just was well with pygresql as with psycopg2.

I have two failing tests in DistinctEngineShardTest in both cases which seem to be unrelated. Shall I create an issue for these?

@zzzeek
Copy link
Owner

zzzeek commented Feb 8, 2016

See the instructions for running the tests in README.unittests.rst. To run tests on any backend, use --dburi: py.test --dburi postgresql+pygresql://foo:bar@bat.

The DistinctEngineShardTest is a really old set of tests that only run against sqlite. I've not seen those fail in years so just show me the failure and I can likely tell you what's going on.

Note that it requires the latest PyGreSQL 5.0
to pass all tests, a warning is printed otherwise.

Minor change to test.dialect.postgresql.test_types:
- JSON content should not be required to be unicode
  if the dialect doesn't return unicode (Python 2).
Minor change to dialects/postgresql.base:
- Return index names properly casted to unicode
@Cito
Copy link
Contributor Author

Cito commented Feb 9, 2016

Ok, found the instructions and got everything working.

The issue with DistinctEngineShardTest seems to be caused by running the tests on a shared folder with vagrant, due to the vboxfs filesystem. When running from a real filesystem, everything worked properly.

I have run the test suite against a Postgres database with PyGreSQL 5.0 now.

Test results with Python 2:

py.test -x --dburi=postgresql+pygresql:///test

============================= test session starts ==============================
platform linux2 -- Python 2.7.6, pytest-2.8.7, py-1.4.31, pluggy-0.3.1 -- /home/vagrant/venv2-sqlalchemy/bin/python
cachedir: .cache
rootdir: /home/vagrant/sqlalchemy, inifile: setup.cfg
collecting ... collected 7577 items

test/aaa_profiling/test_compiler.py::CompileTest_postgresql_pygresql::test_insert <- lib/sqlalchemy/testing/profiling.py SKIPPED
test/aaa_profiling/test_compiler.py::CompileTest_postgresql_pygresql::test_select SKIPPED
test/aaa_profiling/test_compiler.py::CompileTest_postgresql_pygresql::test_select_labels SKIPPED
...
test/sql/test_update.py::UpdateTest::test_update_preserve_order_reqs_listtups PASSED
test/sql/test_update.py::UpdateTest::test_update_to_expression PASSED
test/sql/test_update.py::UpdateTest::test_where_empty PASSED

================== 7052 passed, 525 skipped in 405.45 seconds ==================

Test results with Python 3:

py.test -x --dburi=postgresql+pygresql:///test

============================= test session starts ==============================
platform linux -- Python 3.4.3, pytest-2.8.7, py-1.4.31, pluggy-0.3.1 -- /home/vagrant/venv3-sqlalchemy/bin/python3
cachedir: .cache
rootdir: /home/vagrant/sqlalchemy, inifile: setup.cfg
collecting ... collected 7572 items

test/aaa_profiling/test_compiler.py::CompileTest_postgresql_pygresql::test_insert SKIPPED
test/aaa_profiling/test_compiler.py::CompileTest_postgresql_pygresql::test_select SKIPPED
test/aaa_profiling/test_compiler.py::CompileTest_postgresql_pygresql::test_select_labels SKIPPED
...
test/sql/test_update.py::UpdateTest::test_update_preserve_order_reqs_listtups PASSED
test/sql/test_update.py::UpdateTest::test_update_to_expression PASSED
test/sql/test_update.py::UpdateTest::test_where_empty PASSED

================== 7071 passed, 501 skipped in 405.59 seconds ==================

@zzzeek
Copy link
Owner

zzzeek commented Feb 9, 2016

great! I'd merge this in for 1.1 which is hopefully in the spring, is that OK? (we've gone for ten years w/o a pygresql dialect so far whats another few months ? :) )

@Cito
Copy link
Contributor Author

Cito commented Feb 9, 2016

That's ok. A few months mean nothing to Pygres. It's already 21 years old :)

@zzzeek
Copy link
Owner

zzzeek commented Feb 18, 2016

I likely will have to accept from #172 instead because I closed this person's PR about 8 months ago.

@zzzeek
Copy link
Owner

zzzeek commented Feb 18, 2016

pygresql still inexplicably has private mailing list archives.

@zzzeek
Copy link
Owner

zzzeek commented Apr 12, 2016

Dear contributor -

This pull request is being moved to Gerrit, at https://gerrit.sqlalchemy.org/36, where it may be tested and reviewed more closely. As such, the pull request itself is being marked "closed" or "declined", however your contribution is merely being moved to our central review system. Please register at https://gerrit.sqlalchemy.org#/register/ to send and receive comments regarding this item.

@zzzeek zzzeek closed this Apr 12, 2016
zzzeek pushed a commit that referenced this pull request Apr 15, 2016
Change-Id: I040b75ff3b4110e7e8b26442a4eb226ba8c26715
Pull-request: #234
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

Successfully merging this pull request may close these issues.

2 participants