Skip to content

Conversation

@dlax
Copy link
Contributor

@dlax dlax commented Oct 4, 2022

Prior to this change, the following code:

with conn.pipeline():
   conn.execute("select 'x'").fetchone()
   conn.execute("select 'y'")

would produce the following libpq trace:

F	13	Parse	 "" "BEGIN" 0
F	14	Bind	 "" "" 0 0 1 0
F	6	Describe	 P ""
F	9	Execute	 "" 0
F	18	Parse	 "" "select 'x'" 0
F	14	Bind	 "" "" 0 0 1 0
F	6	Describe	 P ""
F	9	Execute	 "" 0
F	4	Flush
B	4	ParseComplete
B	4	BindComplete
B	4	NoData
B	10	CommandComplete	 "BEGIN"
B	4	ParseComplete
B	4	BindComplete
B	33	RowDescription	 1 "?column?" NNNN 0 NNNN 65535 -1 0
B	11	DataRow	 1 1 'x'
B	13	CommandComplete	 "SELECT 1"
F	13	Parse	 "" "BEGIN" 0
F	14	Bind	 "" "" 0 0 1 0
F	6	Describe	 P ""
F	9	Execute	 "" 0
F	18	Parse	 "" "select 'y'" 0
F	14	Bind	 "" "" 0 0 1 0
F	6	Describe	 P ""
F	9	Execute	 "" 0
B	4	ParseComplete
B	4	BindComplete
B	4	NoData
B	NN	NoticeResponse	 S "WARNING" V "WARNING" C "25001" M "there is already a transaction in progress" F "SSSS" L "SSSS" R "SSSS" \x00
F	4	Sync
B	10	CommandComplete	 "BEGIN"
B	4	ParseComplete
B	4	BindComplete
B	33	RowDescription	 1 "?column?" NNNN 0 NNNN 65535 -1 0
B	11	DataRow	 1 1 'y'
B	13	CommandComplete	 "SELECT 1"
B	5	ReadyForQuery	 T

where we can see that the BEGIN statement (due to the connection being in non-autocommit mode) is emitted twice, as notified by the server.

This is because the transaction state after the implicit BEGIN is not "correct" (i.e. should be INTRANS, but is IDLE) since the result from respective statement has not been received yet.

By syncing after the BEGIN, we fetch result from this command thus get the transaction state INTRANS for following queries. This is similar to what happens with explicit transaction, by using nested pipelines.

@dvarrazzo
Copy link
Member

Just in time: I was about to release 3.1.3. I'll wait for this MR if you think it's important, ok @dlax?

Thank you very much!!!

@dlax
Copy link
Contributor Author

dlax commented Oct 4, 2022

Yes, it's ok on my side. Just waiting on the CI, in case there's any surprise...

Prior to this change, the following code:

    with conn.pipeline():
       conn.execute("select 'x'").fetchone()
       conn.execute("select 'y'")

would produce the following libpq trace:

    F	13	Parse	 "" "BEGIN" 0
    F	14	Bind	 "" "" 0 0 1 0
    F	6	Describe	 P ""
    F	9	Execute	 "" 0
    F	18	Parse	 "" "select 'x'" 0
    F	14	Bind	 "" "" 0 0 1 0
    F	6	Describe	 P ""
    F	9	Execute	 "" 0
    F	4	Flush
    B	4	ParseComplete
    B	4	BindComplete
    B	4	NoData
    B	10	CommandComplete	 "BEGIN"
    B	4	ParseComplete
    B	4	BindComplete
    B	33	RowDescription	 1 "?column?" NNNN 0 NNNN 65535 -1 0
    B	11	DataRow	 1 1 'x'
    B	13	CommandComplete	 "SELECT 1"
    F	13	Parse	 "" "BEGIN" 0
    F	14	Bind	 "" "" 0 0 1 0
    F	6	Describe	 P ""
    F	9	Execute	 "" 0
    F	18	Parse	 "" "select 'y'" 0
    F	14	Bind	 "" "" 0 0 1 0
    F	6	Describe	 P ""
    F	9	Execute	 "" 0
    B	4	ParseComplete
    B	4	BindComplete
    B	4	NoData
    B	NN	NoticeResponse	 S "WARNING" V "WARNING" C "25001" M "there is already a transaction in progress" F "SSSS" L "SSSS" R "SSSS" \x00
    F	4	Sync
    B	10	CommandComplete	 "BEGIN"
    B	4	ParseComplete
    B	4	BindComplete
    B	33	RowDescription	 1 "?column?" NNNN 0 NNNN 65535 -1 0
    B	11	DataRow	 1 1 'y'
    B	13	CommandComplete	 "SELECT 1"
    B	5	ReadyForQuery	 T

where we can see that the BEGIN statement (due to the connection being
in non-autocommit mode) is emitted twice, as notified by the server.

This is because the transaction state after the implicit BEGIN is not
"correct" (i.e. should be INTRANS, but is IDLE) since the result from
respective statement has not been received yet.

By syncing after the BEGIN, we fetch result from this command thus get
the transaction state INTRANS for following queries. This is similar to
what happens with explicit transaction, by using nested pipelines.
@dlax dlax force-pushed the pipeline-no-extra-begin branch from 8570fb2 to 0903334 Compare October 4, 2022 17:22
@dlax dlax marked this pull request as ready for review October 4, 2022 17:22
@dvarrazzo dvarrazzo merged commit 24b5495 into psycopg:master Oct 4, 2022
@dlax dlax deleted the pipeline-no-extra-begin branch October 5, 2022 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants