-
Notifications
You must be signed in to change notification settings - Fork 109
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
fix: Remove DDL automatically issued by Ibis for Postgres connections #1067
Conversation
/gcbrun |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add an integration test to run test_postgres.py
against a read replica? Not for this PR, thinking if we need a new issue for that or if it is overkill?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks much better to me. Because the code was creating the functions on every execute we would generate a lot of warnings if we went down the isolation_level
route I was investigating.
Aside from my comment about tests your changes LGTM.
Neil,
This fix did not work on spanner postgres adapter. Here is some further
information
Researching the issue with the pgadapter, the notes on Ibis 6.0.0 says
```
- *postgres:* Ibis no longer automatically defines first/last reductions
on connection to the postgres backend. Use DDL shown in
https://wiki.postgresql.org/wiki/First/last_(aggregate) or one of
the pgxn implementations instead.` in
https://github.com/ibis-project/ibis/releases.
- ```Might upgrading to 6.0 (or later) help us - it may be a lot of work
though.
…On Wed, Dec 6, 2023, 1:36 AM Neil Johnson ***@***.***> wrote:
***@***.**** commented on this pull request.
Should we add an integration test to run test_postgres.py against a read
replica? Not for this PR, thinking if we need a new issue for that or if it
is overkill?
—
Reply to this email directly, view it on GitHub
<#1067 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AZK36WPNYHEBPTGJEIOASS3YIA4ADAVCNFSM6AAAAABAIIY2SSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONRXGEYDCNRVHA>
.
You are receiving this because your review was requested.Message ID:
<GoogleCloudPlatform/professional-services-data-validator/pull/1067/review/1767101658
@github.com>
|
I don't think we need a test for a read replica since the Ibis upgrade get's rid of the DDL in v6.0 anyways. But if another similar issue comes up, we can consider it. Thanks for the review! |
Closes #1035
do_connect()
method to removeCREATE FUNCTION
DDL automatically issued by Ibis (here)