sql/pgwire: EXPORT doesn't work as a prepared statement when used with node-pg driver

[jfrconley]

Describe the problem

I am encountering mysterious unwanted transactions implicitly created from prepared statements while using node-pg.
This issue only occurs on v22.x and it prevents me from using the 'export' statement since it cannot be used in a transaction.
In addition, I am seeing long running (several hours) transactions being created for otherwise simple one-off queries.

To Reproduce

I've create a simple example repo that reproduces the problem with CockroachDB 22.1

https://github.com/jfrconley/cockroachdb-mystery-transaction-bleed

The steps are essentially:

1. Run any prepared query
2. Run another prepared query including an export statement
3. Gaze despairingly at the error message

Expected behavior
Prepared statements should not create implicit multi statement transactions

Environment:

• CockroachDB : v22.1
• Client app: NodeJS (16) with pg library (8.7.3)

Additional context
Only fix I have found is rolling back to v21.x since the issue only occurs in v22.x

Jira issue: CRDB-16648

[blathers-crl]

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/bulk-io (found keywords: export)

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]

cc @cockroachdb/bulk-io

[otan]

cc @rafiss i wonder if this is fixed by #82622

[rafiss]

It seems like it could be related. I can pick up the investigation, unless you already are.

[otan]

haven't done anything :)

[otan]

i lied (i was building 22.1 anyway), i'm getting

  ● Demonstrate transaction failure › should run multiple separate statements on a single connection

    error: EXPORT cannot be used inside a multi-statement transaction

      at Parser.parseErrorMessage (node_modules/pg-protocol/src/parser.ts:369:69)
      at Parser.handlePacket (node_modules/pg-protocol/src/parser.ts:188:21)
      at Parser.parse (node_modules/pg-protocol/src/parser.ts:103:30)
      at Socket.<anonymous> (node_modules/pg-protocol/src/index.ts:7:48)

on the origin/release-22.1 branch when running the test suite

[rafiss]

I captured a network trace of what this test is doing.

In bc38925 we added logic so that if a statement was prepared, then immediately executed, it would count as an "auto commit" execution, and the EXPORT would be allowed. It works by detecting if a Sync message occurs right after an Execute message (from the wire protocol: https://www.postgresql.org/docs/14/protocol-flow.html#PROTOCOL-FLOW-EXT-QUERY)

This logic isn't sufficient, since the node-pg driver sends Execute, then Flush, and then a Sync message.

So we probably need something to detect Flush messages during this phase of preparing a statement.

[jfrconley]

@rafiss Thank you for diagnosing! Is there an estimate on when a fix for this could be merged? This is a hard blocker for our CockroachDB rollout since it effectively breaks all prepared queries.

[rafiss]

I don't have a timeline for the fix yet, but I will keep investigating.

it effectively breaks all prepared queries

Can you explain? I'm surprised by that. I thought you were saying it only breaks prepared queries for the EXPORT command.

A workaround in the meantime is to use a simple query (no placeholder arguments) for EXPORT commands.

[jfrconley]

It effectively is putting all of my statements in unending transaction contexts, not sure if this will cause other issues. This just happens to break export more obviously. Every prepared statement that I run shows up in the transaction dashboard with an ever increasing transaction time counter. Could this cause rollback in the event of a node drain / shutdown?

I'll investigate using a simple query for export, but I'm concerned about potential security issues using unprepared statements.

[rafiss]

Oh, the "unending transaction contexts" is a separate issue that definitely sounds like a bug. I wasn't aware you were seeing that. Can you show a screenshot of the transaction dashboard with ever increasing transaction times?

[jfrconley]

Here's an example. This statement is simply for paging through objects with no intended transactions. However, you can see it has a very long transaction time connected to it.

[rafiss]

The mean transaction time definitely looks wrong -- 838488371 seconds is about 26 years. So I don't think the problem is that the transaction timer is ever-increasing, but it's actually more of a calculation issue. We must be subtracting the wrong timestamps somewhere.

[rafiss]

We identified the cause of the incorrect times and it should be getting fixed. See #82894

[jfrconley]

@rafiss Is there a timeline for when the export issue could be fixed? It's blocking our adoption of 22.x

[rafiss]

Unfortunately, the idea I mentioned in #82766 (comment) does not work. node-pg is sending the Execute, Flush, then Sync messages one-by-one rather than sending them as a batch all at once. This means that CockroachDB has no way to verify that there is no other operation in the same implicit transaction after the EXPORT is executed.

This screenshot of the Wireshark capture shows what I mean.

To move forward with the fix, node-pg needs to send those commands in a batch.

The workaround until then is to not use prepared statements for EXPORT. Prepared statements for other queries should still work.