Re: 2.7.2 still problem with readonly/autocommit, was: Changing set_session implementation

On Fri, Aug 11, 2017 at 12:22:55AM +0100, Daniele Varrazzo wrote:

> Building psycopg in debug mode gives you all the information to infer
> its behaviour. Here are redacted outputs.
>
> You want to set a connection in autocommit and change a a transaction
> parameter to a non default. If you change parameter in a non
> autocommit database the state will be kept in the python object and
> applied at begin time:
>
> >>> conn = psycopg2.connect('')
> [26725] psyco_connect: dsn = '', async = 0
>
> >>> cur = conn.cursor()
>
> >>> cur.execute("select 1")
> [26725] pq_execute_command_locked: pgconn = 0x902a6f0, query = BEGIN
> [26725] pq_execute: executing SYNC query: pgconn = 0x902a6f0
> [26725] select 1
>
> >>> conn.rollback()
>
> >>> conn.readonly = False
> [26725] conn_set_session: autocommit 0, isolevel 5, readonly 0, deferrable 2
>
> >>> cur.execute("select 1")
> [26725] pq_execute_command_locked: pgconn = 0x902a6f0, query = BEGIN READ WRITE
> [26725] pq_execute: executing SYNC query: pgconn = 0x902a6f0
> [26725] select 1
>
> >>> conn.rollback()
>
>
> If instead you set the connection in autocommit and try changing the
> session state psycopg will change the session state. This is with a
> database defaulting to readonly:
>
> >>> conn = psycopg2.connect('')
> [26725] psyco_connect: dsn = '', async = 0
>
> >>> conn.autocommit = True
> [26725] conn_set_session: autocommit 1, isolevel 5, readonly 2, deferrable 2
>
> >>> conn.readonly = False
> [26725] pq_set_guc_locked: setting default_transaction_read_only to off
> [26725] pq_execute_command_locked: pgconn = 0x8f84280, query = SET
> default_transaction_read_only TO 'off'
> [26725] conn_set_session: autocommit 1, isolevel 5, readonly 0, deferrable 2
>
> >>> cur = conn.cursor()
> >>> cur.execute("create database k")
> [26725] pq_execute: executing SYNC query: pgconn = 0x8f84280
> [26725] create database k
>
> >>> cur.statusmessage
> 'CREATE DATABASE'
>
> As you can see the SET statement is issued *on setting the readonly
> property* when the connection is autocommit. What will not happen is
> psycopg setting the readonly state when switching autocommit, copying
> it from the internal state to the session state. It won't because this
> would be run three queries when switching to autocommit=True; implicit
> operations in unexpected moments usually spell trouble.

I agree with this rationale and I understand your
explanation. Thanks for taking the time. What I am saying,
however, is that I think I have found a bug. Notice this
sequence in my test script:

conn.autocommit = False
conn.readonly = False
conn.autocommit = True
conn.readonly = False

After this I would assume the connection to be in readwrite
mode (per your explanation, psycopg2 would (also) emit a "set
default_trans...off" when the last readonly=False is given.

However, it does NOT, as the PostgreSQL log snippet shows.

Here is the console output (I will check whether I can get
access to a debug build):

conn: <connection object at 0xb66a88b4; dsn: 'dbname=gnumed_v20 user=gm-dbo', closed: 0>
readonly: None
autocommit: False
setting autocommit to False
autocommit now: False
setting readonly to False
readonly now: False
setting autocommit to True
autocommit now: True
readonly now: False
querying DEFAULT_TRANSACTION_READ_ONLY state (should show "ON")
[('DATABASE', 'default_transaction_read_only', 'ON')]
the following SQL will fail: create database gnumed_v20_copy template gnumed_v20
(note that the transaction being talked about is implicit to PostgreSQL, due to autocommit mode)
SQL failed:
cannot execute CREATE DATABASE in a read-only transaction
shutting down

If I comment out the *first* call to readonly=False (after
autocommit=False, before autocommit=True) it does work.

It seems as if psycopg2 "remembers" that it has already
called readonly=False.

Here is the script again:

db = u'gnumed_v20' # a database configured "alter database %s set default_transaction_read_only to on"
user = 'gm-dbo' # a user with CREATE DATABASE powers
cmd_def_tx_ro = "SELECT upper(source), name, upper(setting) FROM pg_settings WHERE name = 'default_transaction_read_only'"
cmd_create_db = "create database %s_copy template %s" % (db, db)
import sys
import psycopg2
conn = psycopg2.connect(dbname = db, user = user)
print 'conn:', conn
print 'readonly:', conn.readonly
print 'autocommit:', conn.autocommit
print 'setting autocommit to False'
conn.autocommit = False
print 'autocommit now:', conn.autocommit
print 'setting readonly to False'
conn.readonly = False # comment out for success
print 'readonly now:', conn.readonly
print 'setting autocommit to True'
conn.autocommit = True
print 'autocommit now:', conn.autocommit
conn.readonly = False
print 'readonly now:', conn.readonly
curs = conn.cursor()
curs.execute(cmd_def_tx_ro)
print 'querying DEFAULT_TRANSACTION_READ_ONLY state (should show "ON")'
print curs.fetchall()
curs.close()
conn.commit()
print 'the following SQL will fail:', cmd_create_db
print '(note that the transaction being talked about is implicit to PostgreSQL, due to autocommit mode)'
curs = conn.cursor()
try:
curs.execute(cmd_create_db)
except psycopg2.InternalError as ex:
print 'SQL failed:'
print ex
print 'shutting down'
curs.close()
conn.rollback()
conn.close()

> This should be enough to work around your problem: put the connection
> in read-write state *after* setting autocommit

I do, as you can see from the script above.

> or execute "SET
> default_transaction_read_only TO 'off'" manually before creating the
> database if for some reason in your workflow you cannot do that.

This is the workaround I am currently using.
I still wonder why the above does not work.

> Hope this helps. Feel free to propose a merge request with change in
> documentation if you find it would have helped.

At this point I am still unsure whether the above is a bug.

The documentation change would amount to something like

once in autocommit mode do not rely on psycopg2 methods
for switching readonly on or off, do not even rely on
what the psycopg2 connection object _tells_ you about the
state it is in

While it seems true at the moment I don't think this goes
along with the overall excellent quality of psycopg2 :-)

May I kindly ask you to run my script against a readonly
database ? The particular dance between autocommit/readonly
on/off is needed to show the problem.

Meanwhile I will try to somehow run a debug build.

Thanks for thinking along,
Karsten
--
GPG key ID E4071346 @ eu.pool.sks-keyservers.net
E167 67FD A291 2BEA 73BD 4537 78B9 A9F9 E407 1346