-
Notifications
You must be signed in to change notification settings - Fork 409
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 default_transaction_isolation on system/database level #8312
base: master
Are you sure you want to change the base?
Conversation
edb/edgeql/compiler/config.py
Outdated
# Special case for default_transaction_isolation: | ||
# Gel default is `serializable`, while PG default is `read committed`. |
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.
Call this out as a HACK
in the comments.
Some day we may want this to be a general mechanism?
(Not today.)
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.
Yeah, now that I'm thinking more about it, maybe we should always reset the system backend settings to their Gel defaults explicitly, as we cannot rely on the backend defaults, like users could put anything in postgresql.conf
and run Gel with --backend-dsn
.
Actually wait, I don't fully understand; when does |
During bootstrap, backend settings that have defaults are applied to the PG config file? |
Roger, got it. I had forgotten that step but on reading the code it seems familiar. |
Wait --- cloud doesn't have configfile access, right? |
client
GucSource (backend connection parameter) has higher priority thandatabase
orconfiguration file
(system). Now thatdefault_transaction_isolation
is exposed, it has to be removed from the backend connection parameters. Instead, it now becomes a permanent system config that always has a value.This breaks remote backends without config file access unless they are preconfigured with
default_transaction_isolation = serializable
system-wide.Refs #8276