-
Notifications
You must be signed in to change notification settings - Fork 86
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
RuntimeError: Cannot close AdbcConnection with open AdbcStatement #538
Comments
I suppose you might want to pin 0.2.0 for now. Is there more context? The Polars code in question appears to be closing the cursor correctly (though it should use a context manager), so I think this would only happen if there was an uncaught exception raised. |
Unfortunately, it is not working with pinned version. And there seems to be something wrong with the Polars code. It is doing:
Shouldn't it be somehting like (or with context manager as I am posting in the code below)
With the below workaround I got a different error, which I can't find any context on. Can you help with that? The error is:
The text column in db is varchar(16)
Any help is really appreciated!! |
Unfortunately, it is not working with pinned version. And there seems to be something wrong with the Polars code. It is doing:
Shouldn't it be somehting like (or with context manager as I am posting in the code below)
With the below workaround I got a different error, which I can't find any context on. Can you help with that? The error is:
The text column in db is varchar(16)
Any help is really appreciated!! |
The driver is fairly new and I haven't yet had time to build out support for all the different Postgres types and do benchmarking. So right now, string columns just aren't supported yet, sorry :/ |
That's fair - thanks! Is there an open "issue" I can track to see when the support is added? |
We can leave this issue open too, I'll use this to also add integration tests with Polars (I hadn't realized they had done this - so thank you for bringing it up to me) and see if I can reproduce things/improve things on our side and Polars side (context manager, and supporting the "fail" mode they list) |
Hi @lidavidm is there any update on this? I see integration tests for writing to the database but not reading; I'm unable to do even a simple query in polars via postgres (note it claims syntax error but there is none in my query):
|
What is the full query it generated? |
It was: SELECT string_field
FROM my_table
WHERE date_field = '2023-05-10'
ORDER BY string_field |
Hmm, I'm not able to replicate this. I created a table: postgres=# CREATE TABLE my_table (string_field TEXT, date_field DATE);
CREATE TABLE
postgres=# INSERT INTO my_table VALUES ('a', '2023-05-10'), ('b', '2023-05-11'), ('c', '2023-05-12');
INSERT 0 3
postgres=# SELECT * FROM my_table;
string_field | date_field
--------------+------------
a | 2023-05-10
b | 2023-05-11
c | 2023-05-12
(3 rows) Then I ran your query with polars: >>> import polars
>>> import polars as pl
>>> uri = 'postgres://localhost:5432/postgres?user=postgres&password=password'
>>> query = '''SELECT string_field
... FROM my_table
... WHERE date_field = '2023-05-10'
... ORDER BY string_field'''
>>> pl.read_database(query, connection_uri=uri, engine='adbc')
shape: (1, 1)
┌──────────────┐
│ string_field │
│ --- │
│ str │
╞══════════════╡
│ a │
└──────────────┘
>>> pl.__version__
'0.18.4' |
Given the error, it seems something differs about the query?
|
What's your system? I'm running on Windows w/ Greenplum postgres, which I think may be the issue here. I set up the exact same table as you with the exact same query and I still get the error: uri = f"postgresql://{username}:{password}@{server}:5432/{database}"
query = "SELECT string_field FROM my_table WHERE date_field = '2023-05-10'"
pl.read_database(query, connection_uri=uri, engine="adbc")
|
Hmm, Ubuntu Linux with PostgreSQL 13. Does Greenplum's fork retain COPY support? If not, the driver won't work. |
e.g. does this work in psql?
|
Ahh, that's the issue. postgres
I've run into the past. The workaround is to use the
And in the
|
Right, but we are using COPY TO STDOUT as seen in the error message. |
Thanks for your help David. I think the issue is in the
When I run this in my handy DBeaver postgresql app, I get this error: But note that when I run without the |
Hi David, I found the Greenplum documentation. My company is still on Greenplum 5 (6 is the latest, 7 in beta). It looks like in 6+ they support the So, yeah. Is there an easy way to get around this by altering the |
By the way, this looks like it was a change in Postgres between Version 8.4, which didn't use the word This changed all the way back in 2010, so I'm guessing there's no much hope for the legacy support/syntax, and it probably wouldn't even return the correct format anyway. Thanks and I'm glad we were able to figure out my issue. |
There's not a way to adjust this (and postgres 8 is long EOL). Even if there was, the COPY mode is quite tied into postgres internals so there's no guarantee we can properly read the data. It's possible we could eventually support falling back to the non-COPY mode (we already have to do this with prepared statements - you could experiment with that?). |
Right, I figured. I think I'll have to continue to use |
I'm going to close this now - we added some integration tests with Polars and we now test with more Postgres versions, though still not version 8 (and the driver probably still won't work with that version). |
I am using polars library to write to Azure Postgre SQL DB.
This is my code:
This is the error:
Versions I have installed:
Please let me know if you need anything further.
Many thanks!
The text was updated successfully, but these errors were encountered: