sql, sqlstats: create temporary stats container for all txns

[xinhaoz]

Commit 1

sql: add txn_fingerprint_id to node_statement_statistics

This commit adds the txn_fingerprint_id column to
crdb_internal.node_statement_statistics.

Release note (sql): txn_fingerprin_id has been added to
crdb_internal.node_statement_statistics. The type of the
column is NULL or STRING.

---

Commit 2

sql, sqlstats: create temporary stats container for all txns

Fixes: #81470

Previously, the stats collector followed different procedures for stats
collection depending on whether or not the txn was explicit.

For explicit transactions, all the stmts in the txn must be recorded with
the same transactionFingerprintID, which is only known after all stmts
in the txn have been executed. In order to record the correct
txnFingerprintID, a temporary stats container was created for stmts in the
current transaction. The transactionFingerprintID was then populated for
all stmts in the temp container, and the temp container was merged with
the parent.

For implict transactions, the assumption was there would only be a single
stmt in the txn, and so no temporary container was created, with stmts
being written directly to the application stats.

This assumption was incorrect, as it is possible for implicit txns to have
multiple stmts, such as stmts sent in a batch.

This commit ensures that stats are properly collected for implicit txns
with multiple stmts. The stats collector now follows the same procedure
for both explicit and implicit txns, creating a temporary container for
local txn stmts and merging on txn finish.

Release note (bug fix): Statement and transaction stats are now properly
recorded for implicit txns with multiple stmts.

[cockroach-teamcity]

This change is 

[rafiss]

nice! the connExecutor changes lgtm. the sqlstats package changes also lgtm, but i haven't read that code before

and just to confirm, the second commit can be backported to v22.1, right?

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss and @xinhaoz)

---

-- commits line 7 at r1:
nit: typo in the name

[maryliag]

And the goal is indeed backport this to 22.1

Reviewed 4 of 4 files at r1, 8 of 8 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @xinhaoz)

[xinhaoz]

One test case in the 2nd commit needs the first commit. To clarify, it's okay to backport both right? Or are we aiming for just the 2nd one with the test case removed?

[matthewtodd]

Nice! Thank you for making this code simpler and easier to understand!

Re: what to backport, are these 2 commits here together because they're related? FWICT, it doesn't seem that they are. Is it to avoid a second (long!) CI cycle? Given the user-visible new column in the first commit, it seems better to me to just backport the second one (modifying the test), but maybe @maryliag can weigh in on whether that hygiene's worth the extra effort.

[xinhaoz]

I'll bors and check with Marylia to confirm backports. Thank you for the reviews!
bors r+

[craig]

Build succeeded:

• GitHub CI (Cockroach)

[blathers-crl]

Encountered an error creating backports. Some common things that can go wrong:

1. The backport branch might have already existed.
2. There was a merge conflict.
3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

---

error creating merge commit from 607026a to blathers/backport-release-22.1-83616: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.1.x failed. See errors above.

---

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