Hey!
Oh, I completely missed this fact from the previous thread! Thanks for pointing this out!Hi,It would be really great to get this field in, but I think the current implementation still suffers from the same issue that is mentioned here [1]. We cannot rely on GetCurrentStatementStartTimestamp() in-line because ExecutorEnd is deferred to the next execution in the case of extended query protocol.
I agree this might be the proper way to go but I don't want to touch core functionality in this patch. I'm afraid it could lead to even more problems with accepting.I think we need to add it to track the start timestamp in queryDesc. What do you think?
Please, find my new patch that addresses the issue. It captures the statement start timestamp at ExecutorStart and then save it to per-nesting-level array to write it later in ExecutorEnd. I set the max nesting level to 64, just because it feels right. I have no other motivation here. For even deeper statements it will fall back to direct GetCurrentStatementStartTimestamp().
I added new regression test based on your example.Benchmark is still OK (16-vCPU, pgbench -c8 -j4 -T60, explicit transactions with
15 SELECT statements each): master HEAD: ~4356 TPS (runs: 4089, 4308, 4672) patched v2: ~4383 TPS (runs: 4226, 4382, 4541) difference: ~0.6% How do you feel about it? Best regards, Pavlo Golub
v2-0001-pg_stat_statements-Add-last_execution_start-colum.patch
Description: Binary data
