On Mon, 31 Oct 2022 at 07:36, Dave Page <dp...@pgadmin.org> wrote: > FYI, this is not intentional, and I do plan to look into it, however I've > been somewhat busy with pgconfeu, and am travelling for the rest of this > week as well. >
Here's a patch to fix this issue. Many thanks to Peter Eisentraut who figured it out in a few minutes after I spent far too long looking down rabbit holes in entirely the wrong place. Thanks for the bug report. > > On Sun, 23 Oct 2022 at 21:09, Robert Treat <r...@xzilla.net> wrote: > >> On Fri, Oct 14, 2022 at 2:55 PM Dave Page <dp...@pgadmin.org> wrote: >> > On Fri, 14 Oct 2022 at 19:16, Andres Freund <and...@anarazel.de> wrote: >> >> On 2022-10-13 14:38:06 +0100, Dave Page wrote: >> >> > Thanks for that. It looks good to me, bar one comment (repeated 3 >> times in >> >> > the sql and expected files): >> >> > >> >> > fetch timestamps from before the next test >> >> > >> >> > "from " should be removed. >> >> >> >> I was trying to say something with that from, but clearly it wasn't >> >> understandable :). Removed. >> >> >> >> With that I pushed the changes and marked the CF entry as committed. >> > >> > >> > Thanks! >> > >> >> Hey folks, >> >> I was looking at this a bit further (great addition btw) and noticed >> the following behavior (this is a mre of the original testing that >> uncovered this): >> >> pagila=# select * from pg_stat_user_tables ; >> relid | schemaname | relname | seq_scan | last_seq_scan | >> seq_tup_read | idx_scan | last_idx_scan | idx_tup_fetch | n_tup_ins | >> n_tup_upd | n_tup_del | n_tup_hot_upd | n_live_tup | n_dead_tup | >> n_mod_since_analyze | n_ins_since_vacuum | last_vacuum | >> last_autovacuum | last_analyze | last_autoanalyze | vacuum_count | >> autovacuum_count | analyze_count | autoanalyze_count >> >> -------+------------+---------+----------+---------------+--------------+----------+---------------+---------------+-----------+-----------+-----------+---------------+------------+------------+---------------------+--------------------+-------------+-----------------+--------------+------------------+--------------+------------------+---------------+------------------- >> (0 rows) >> >> pagila=# create table x (xx int); >> CREATE TABLE >> Time: 2.145 ms >> pagila=# select * from pg_stat_user_tables ; >> relid | schemaname | relname | seq_scan | last_seq_scan | >> seq_tup_read | idx_scan | last_idx_scan | idx_tup_fetch | n_tup_ins | >> n_tup_upd | n_tup_del | n_tup_hot_upd | n_live_tup | n_dead_tup | >> n_mod_since_analyze | n_ins_since_vacuum | last_vacuum | >> last_autovacuum | last_analyze | last_autoanalyze | vacuum_count | >> autovacuum_count | analyze_count | autoanalyze_count >> >> -------+------------+---------+----------+---------------+--------------+----------+---------------+---------------+-----------+-----------+-----------+---------------+------------+------------+---------------------+--------------------+-------------+-----------------+--------------+------------------+--------------+------------------+---------------+------------------- >> 16392 | public | x | 0 | [null] | >> 0 | [null] | [null] | [null] | 0 | 0 | >> 0 | 0 | 0 | 0 | >> 0 | 0 | [null] | [null] | [null] >> | [null] | 0 | 0 | 0 | >> 0 >> (1 row) >> >> pagila=# insert into x select 1; >> INSERT 0 1 >> pagila=# select * from pg_stat_user_tables ; >> relid | schemaname | relname | seq_scan | last_seq_scan | >> seq_tup_read | idx_scan | last_idx_scan | idx_tup_fetch | n_tup_ins | >> n_tup_upd | n_tup_del | n_tup_hot_upd | n_live_tup | n_dead_tup | >> n_mod_since_analyze | n_ins_since_vacuum | last_vacuum | >> last_autovacuum | last_analyze | last_autoanalyze | vacuum_count | >> autovacuum_count | analyze_count | autoanalyze_count >> >> -------+------------+---------+----------+------------------------+--------------+----------+---------------+---------------+-----------+-----------+-----------+---------------+------------+------------+---------------------+--------------------+-------------+-----------------+--------------+------------------+--------------+------------------+---------------+------------------- >> 16392 | public | x | 0 | 1999-12-31 19:00:00-05 | >> 0 | [null] | [null] | [null] | 1 | >> 0 | 0 | 0 | 1 | 0 | >> 1 | 1 | [null] | [null] | >> [null] | [null] | 0 | 0 | >> 0 | 0 >> (1 row) >> >> Normally we populate "last" columns with a NULL value when the >> corresponding marker is zero, which seems correct in the first query, >> but no longer matches in the second. I can see an argument that this >> is a necessary exception to that rule (I'm not sure I agree with it, >> but I see it) but even in that scenario, ISTM we should avoid >> populating the table with a "special value", which generally goes >> against observability best practices, and I believe we've been able to >> avoid it elsewhere. >> >> Beyond that, I also notice the behavior changes when adding a table >> with a PK, though not necessarily better... >> >> pagila=# drop table x; >> DROP TABLE >> Time: 2.896 ms >> pagila=# select * from pg_stat_user_tables ; >> relid | schemaname | relname | seq_scan | last_seq_scan | >> seq_tup_read | idx_scan | last_idx_scan | idx_tup_fetch | n_tup_ins | >> n_tup_upd | n_tup_del | n_tup_hot_upd | n_live_tup | n_dead_tup | >> n_mod_since_analyze | n_ins_since_vacuum | last_vacuum | >> last_autovacuum | last_analyze | last_autoanalyze | vacuum_count | >> autovacuum_count | analyze_count | autoanalyze_count >> >> -------+------------+---------+----------+---------------+--------------+----------+---------------+---------------+-----------+-----------+-----------+---------------+------------+------------+---------------------+--------------------+-------------+-----------------+--------------+------------------+--------------+------------------+---------------+------------------- >> (0 rows) >> >> pagila=# create table x (xx int primary key) ; >> CREATE TABLE >> >> pagila=# select * from pg_stat_user_tables ; >> relid | schemaname | relname | seq_scan | last_seq_scan >> | seq_tup_read | idx_scan | last_idx_scan | idx_tup_fetch | >> n_tup_ins | n_tup_upd | n_tup_del | n_tup_hot_upd | n_live_tup | >> n_dead_tup | n_mod_since_analyze | n_ins_since_vacuum | last_vacuum | >> last_autovacuum | last_analyze | last_autoanalyze | vacuum_count | >> autovacuum_count | analyze_count | autoanalyze_count >> >> -------+------------+---------+----------+-------------------------------+--------------+----------+---------------+---------------+-----------+-----------+-----------+---------------+------------+------------+---------------------+--------------------+-------------+-----------------+--------------+------------------+--------------+------------------+---------------+------------------- >> 16400 | public | x | 1 | 2022-10-23 >> 15:53:04.935192-04 | 0 | 0 | [null] | >> 0 | 0 | 0 | 0 | 0 | 0 >> | 0 | 0 | 0 | [null] >> | [null] | [null] | [null] | 0 | >> 0 | 0 | 0 >> (1 row) >> >> pagila=# insert into x select 1; >> INSERT 0 1 >> >> pagila=# select * from pg_stat_user_tables ; >> relid | schemaname | relname | seq_scan | last_seq_scan >> | seq_tup_read | idx_scan | last_idx_scan | idx_tup_fetch | >> n_tup_ins | n_tup_upd | n_tup_del | n_tup_hot_upd | n_live_tup | >> n_dead_tup | n_mod_since_analyze | n_ins_since_vacuum | last_vacuum | >> last_autovacuum | last_analyze | last_autoanalyze | vacuum_count | >> autovacuum_count | analyze_count | autoanalyze_count >> >> -------+------------+---------+----------+-------------------------------+--------------+----------+------------------------+---------------+-----------+-----------+-----------+---------------+------------+------------+---------------------+--------------------+-------------+-----------------+--------------+------------------+--------------+------------------+---------------+------------------- >> 16400 | public | x | 1 | 2022-10-23 >> 15:53:04.935192-04 | 0 | 0 | 1999-12-31 19:00:00-05 >> | 0 | 1 | 0 | 0 | 0 | >> 1 | 0 | 1 | 1 | >> [null] | [null] | [null] | [null] | >> 0 | 0 | 0 | 0 >> (1 row) >> >> This time the create table both populate a sequential scan and >> populates the last_seq_scan with a real/correct value. However an >> insert into the table neither advances the seq_scan nor the >> last_seq_scan values which seems like different behavior from my >> original example, with the added negative that the last_idx_scan is >> now populated with a special value :-( >> >> I think the simplest fix which should correspond to existing versions >> behavior would be to just ensure that we replace any "special value" >> timestamps with a real transaction timestamp, and then maybe note that >> these fields may be advanced by operations which don't strictly show >> up as a sequential or index scan. >> >> >> Robert Treat >> https://xzilla.net >> > > > -- > Dave Page > Blog: https://pgsnake.blogspot.com > Twitter: @pgsnake > > EDB: https://www.enterprisedb.com > > -- Dave Page Blog: https://pgsnake.blogspot.com Twitter: @pgsnake EDB: https://www.enterprisedb.com
fix_no_scan_handling.diff
Description: Binary data