Re: RFC: Allow EXPLAIN to Output Page Fault Information

The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: tested, failed
Spec compliant: not tested
Documentation: not tested

Hi,

I looked at v11, focusing on structured EXPLAIN output and a small local
build/test pass.

The patch applies cleanly on current master at
db5ed03217b9c238703df8b4b286115d6e940488, but git am warned about trailing
whitespace. git diff --check origin/master...HEAD reports:

src/test/modules/test_misc/t/011_explain_storage_io.pl:47: trailing whitespace
src/test/modules/test_misc/t/011_explain_storage_io.pl:55: trailing whitespace
src/test/modules/test_misc/t/011_explain_storage_io.pl:61: trailing whitespace

A second thing I noticed is that, with io_method=sync, structured EXPLAIN
output can show an Execution Storage I/O section even when ANALYZE is not
used.

For example:

CREATE TABLE t AS SELECT generate_series(1, 3) AS a;
EXPLAIN (BUFFERS, FORMAT JSON) SELECT * FROM t;

returns, in part:

"Planning": {
...
"Storage I/O Read": 0,
"Storage I/O Write": 0
},
"Execution": {
"Storage I/O Read": 0,
"Storage I/O Write": 0
}

The same happens with FORMAT XML and FORMAT YAML. The query was not
executed, so I don't think an Execution section should be emitted here.

The existing BUFFERS documentation says that when BUFFERS is true without
ANALYZE, only buffer usage during query planning is reported. Since this
output is under BUFFERS, an Execution Storage I/O section without ANALYZE
looks surprising to me.

For comparison, text output for the same query reports only the Planning
section and has no Execution section:

Planning:
Buffers: shared hit=12 read=7

It looks like this comes from ExplainOnePlan() considering execution Storage
I/O whenever es->buffers is true, and peek_storageio_usage() returning true
for non-text formats even when both counters are zero. Should the execution
Storage I/O section be gated on es->analyze, similar to Execution Time?

Local checks I ran:

git diff --check origin/master...HEAD: failed with the whitespace issue above
./configure --prefix="$PWD/pg-install" --without-readline --without-zlib --without-icu: passed
make -s -j8: passed
make -s install: passed
make -C src/test/regress check TESTS=explain: passed

I tried to run the new TAP test, but this local environment is missing the
IPC::Run Perl module required by --enable-tap-tests, so I could not verify it
locally.

I could not test Linux io_uring behavior in this local macOS environment, and
I have not benchmarked the getrusage() overhead.

Regards,
Ilmar Yunusov

The new status of this patch is: Waiting on Author