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

Transaction not read only if BEGIN READ ONLY is executed on advanced protocol #87012

Closed
dvarrazzo opened this issue Aug 29, 2022 · 1 comment · Fixed by #87047
Closed

Transaction not read only if BEGIN READ ONLY is executed on advanced protocol #87012

dvarrazzo opened this issue Aug 29, 2022 · 1 comment · Fixed by #87047
Assignees
Labels
A-tools-psycopg C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community X-blathers-triaged blathers was able to find an owner

Comments

@dvarrazzo
Copy link

dvarrazzo commented Aug 29, 2022

Describe the problem

Issue found after fixing psycopg/psycopg#350. In order to execute the BEGIN to start a transaction, we used to use the libpq function PQexec(), but we had to switch to using PQexecParams() because of a bug in libpq 14.5.

If a transaction is started using PQexecParams("BEGIN READ ONLY", None), the transaction starts, but it won't be in read only mode.

To Reproduce

Run crdb:

docker run --rm --name crdb -p 26257:26257 cockroachdb/cockroach:v22.1.6 start-single-node --insecure

Run the following script using psycopg 3.1 or checking out master:

# bug-crdb-read-only.py

from psycopg import pq

DSN = "host=127.0.0.1 user=root port=26257 dbname=defaultdb"

conn = pq.PGconn.connect(DSN.encode())
conn.trace(1)
conn.set_trace_flags(pq.Trace.SUPPRESS_TIMESTAMPS | pq.Trace.REGRESS_MODE)

print("ok")
assert conn.transaction_status == pq.TransactionStatus.IDLE
conn.exec_(b"BEGIN READ ONLY")
assert conn.transaction_status == pq.TransactionStatus.INTRANS
res = conn.exec_(b"SHOW transaction_read_only")
assert res.get_value(0, 0) == b"on", res.get_value(0, 0)
conn.exec_(b"ROLLBACK")

print("bad")
assert conn.transaction_status == pq.TransactionStatus.IDLE
conn.exec_params(b"BEGIN READ ONLY", None)
assert conn.transaction_status == pq.TransactionStatus.INTRANS
res = conn.exec_(b"SHOW transaction_read_only")
assert res.get_value(0, 0) == b"on", res.get_value(0, 0)

conn.finish()

The script passes on CRDB 21.2.14. On 22.1.6, it fails with output:

ok
F	20	Query	 "BEGIN READ ONLY"
B	10	CommandComplete	 "BEGIN"
B	5	ReadyForQuery	 T
F	31	Query	 "SHOW transaction_read_only"
B	46	RowDescription	 1 "transaction_read_only" NNNN 2 NNNN 65535 -1 0
B	12	DataRow	 1 2 'on'
B	11	CommandComplete	 "SHOW 1"
B	5	ReadyForQuery	 T
F	13	Query	 "ROLLBACK"
B	13	CommandComplete	 "ROLLBACK"
B	5	ReadyForQuery	 I
bad
F	23	Parse	 "" "BEGIN READ ONLY" 0
F	14	Bind	 "" "" 0 0 1 0
F	6	Describe	 P ""
F	9	Execute	 "" 0
F	4	Sync
B	4	ParseComplete
B	4	BindComplete
B	4	NoData
B	10	CommandComplete	 "BEGIN"
B	5	ReadyForQuery	 T
F	31	Query	 "SHOW transaction_read_only"
B	46	RowDescription	 1 "transaction_read_only" NNNN 2 NNNN 65535 -1 0
B	13	DataRow	 1 3 'off'
B	11	CommandComplete	 "SHOW 1"
B	5	ReadyForQuery	 T
Traceback (most recent call last):
  File "bug-crdb-read-only.py", line 24, in <module>
    assert res.get_value(0, 0) == b"on", res.get_value(0, 0)
AssertionError: b'off'
F	4	Terminate

Environment:

  • CockroachDB version: 22.1.6 (working in 21.2.14)
  • Server OS: Linux

Epic CRDB-14049
Jira issue: CRDB-19095

@dvarrazzo dvarrazzo added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Aug 29, 2022
@blathers-crl
Copy link

blathers-crl bot commented Aug 29, 2022

Hello, I am Blathers. I am here to help you get the issue triaged.

Hoot - a bug! Though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here.

I have CC'd a few people who may be able to assist you:

  • @cockroachdb/sql-experience (found keywords: libpq,import)

If we have not gotten back to your issue within a few business days, you can try the following:

  • Join our community slack channel and ask on #cockroachdb.
  • Try find someone from here if you know they worked closely on the area and CC them.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@blathers-crl blathers-crl bot added O-community Originated from the community X-blathers-triaged blathers was able to find an owner labels Aug 29, 2022
dvarrazzo added a commit to psycopg/psycopg that referenced this issue Aug 29, 2022
The connection doesn't go in read-only mode if the statement is executed
using the advanced protocol.

See cockroachdb/cockroach#87012
dvarrazzo added a commit to psycopg/psycopg that referenced this issue Aug 29, 2022
The connection doesn't go in read-only mode if the statement is executed
using the advanced protocol.

See cockroachdb/cockroach#87012
dvarrazzo added a commit to psycopg/psycopg that referenced this issue Aug 29, 2022
The connection doesn't go in read-only mode if the statement is executed
using the advanced protocol.

See cockroachdb/cockroach#87012
@rafiss rafiss self-assigned this Aug 29, 2022
@craig craig bot closed this as completed in f4b491f Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tools-psycopg C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community X-blathers-triaged blathers was able to find an owner
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants