Finish handling compatibility with pg14

[rjuju]

Also remove the dual query identifier handling, and some other unrelated small patches.

[rjuju]

One thing that should be discussed: should we keep the SQL function pg_store_plans_hash_query available for pg14+, and what to do with the related documentation, which I didn't change yet.

[horiguti]

Thanks for the update! It looks fine as a whole.

I have some comments on it. "Remove any "Async Capable" info from the textual plan output." p := regexp_replace(p, '[ ]*Async Capable: (true|false)[ ]*\n', '', 'g'); Single concrete characters don't need surrounding brackets. Is there any reason for the brackets?

"Use ROLE_PG_READ_ALL_STATS spelling." Looks fine.

"Remove some trailing whitespaces and other minor stylistic issues." "Move misplaced comment." "Remove sjson2testsql.pl" Looks fine. Thanks.

"Remove the queryid_stat_statements field." We might want to split branches for new versions but also we don't need to bother doing that:p

            values[i++] = Int64GetDatumFast(queryid);
            values[i++] = Int64GetDatumFast(planid);
-           values[i++] = Int64GetDatumFast(queryid_stmt);
+           if (api_version == PGSP_V1_5)
+               values[i++] = Int64GetDatumFast(0);

If someone examines the pre-1.6 field, it is supposed to be the join key with pg_stat_statements. So I think we should show queryid there, even if it is the same with the near-by field.

+       Assert(i == (api_version == PGSP_V1_5 ? PG_STORE_PLANS_COLS_V1_5 :
+                   api_version == PGSP_V1_6 ? PG_STORE_PLANS_COLS_V1_6 :
+                   -1 /* fail if you forget to update this assert */ ));

Indentation is a bit off on the second and third lines.

"Ignore psqlrc file in make rules relying on psql." Looks fine. I was not aware of that..

"Handle new nodes and fields added in pg14." Looks fine, but I want to see the new property node in regtest, as the follows (Sorry, I'm not sure how to do this on github... And it contains a new comment irrelevant to its main purpose..)

diff --git a/json2sql.pl b/json2sql.pl
index e9930c7..fb37e98 100755
--- a/json2sql.pl
+++ b/json2sql.pl
@@ -161,6 +161,9 @@ SELECT '### '||'normalize        '||title||E'\n'||

 EOS

+## Write all possible json properties for round-trip test.  For the
+## reason of the limited length of compressed plans, we need to emit
+## it in multiple pieces.
 sub setplan0 {
    my($addunknown) = @_;
    $plan = << 'EOS';
@@ -333,7 +336,8 @@ sub setplan1 {
   "Sampling Parameters": ["''10''::real"],
   "Repeatable Seed": "''0''::double precision",
   "Workers": "dummy",
-  "Worker Number": 0
+  "Worker Number": 0,
+  "Async Capable": 0
 EOS

 # Avoid trailing new line
-- 
2.27.0

[rjuju]

Thanks for the review!

p := regexp_replace(p, '[ ]*Async Capable: (true|false)[ ]*\n', '', 'g');

Single concrete characters don't need surrounding brackets. Is there any reason for the brackets?

The brackets there are used as a bracket expression. In order to remove only the additional content in pg14 I need to keep the previous newline and remove only one newline after the Async Capable part. It means that I can't use something like \s* but only the space character, so a bracket expression is used. I could use a subexpression instead but those are usually more costly even if the subexpression is not referenced (I think this was changed a few months back, but still).

Edit: ah yes the bracket expression isn't mandatory, but it somehow feels less ambiguous to me and I wasn't sure at first if I would need other characters like tabulation. I can drop if it you prefer.

However clearly what this regexp does and why isn't obvious, as the text format isn't straightforwar eitherd, so I will add some more explanations!

If someone examines the pre-1.6 field, it is supposed to be the join key with pg_stat_statements. So I think we should show queryid there, even if it is the same with the near-by field.

Ah indeed it would be way better. While on that topic, should we return NULL rather than a 0 for both queryid and planid?

Indentation is a bit off on the second and third lines.

Indeed, I will fix!

"Handle new nodes and fields added in pg14." Looks fine, but I want to see the new property node in regtest, as the follows

Ok, I will fix!

[rjuju]

I just pushed all requested fixes!

[rjuju]

As discussed offline it's time to work on a clean patchset. I'm therefore closing this PR so I can integrate @yamatattsu changes in a single commit.

Thanks a lot for the review!