Re: v13: show extended stats target in \d

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

Hi,

I will humbly disagree with the current review. I shall refrain from changing the status though because I do not have very strong feeling about it.

I am in agreement that it is a helpful feature and as implemented, the result seems to be the desired one.

However the patch contains:

-                                                         "  'm' = any(stxkind) AS mcv_enabled\n"
+                                                         "  'm' = any(stxkind) AS mcv_enabled,\n"
+                                                         "  %s"
                                                          "FROM pg_catalog.pg_statistic_ext stat "
                                                          "WHERE stxrelid = '%s'\n"
                                                          "ORDER BY 1;",
+                                                         (pset.sversion >= 130000 ? "stxstattarget\n" : "-1\n"),
                                                          oid);

This seems to be breaking a bit the pattern in describeOneTableDetails().
First, it is customary to write the whole query for the version in its own block. I do find this pattern rather helpful and clean. So in my humble opinion, the ternary expression should be replaced with a distinct if block that would implement stxstattarget. Second, setting the value to -1 as an indication of absence breaks the pattern a bit further. More on that bellow.

+                                       if (strcmp(PQgetvalue(result, i, 8), "-1") != 0)
+                                               appendPQExpBuffer(&buf, " STATISTICS %s",
+                                                                         PQgetvalue(result, i, 8));
+

In the same function, one will find a bit of a different pattern for printing the statistics, e.g.
     if (strcmp(PQgetvalue(result, i, 7), "t") == 0) 
I will again hold the opinion that if the query gets crafted a bit differently, one can inspect if the table has `stxstattarget` and then, go ahead and print the value.

Something in the lines of:
                    if (strcmp(PQgetvalue(result, i, 8), "t") == 0)
                        appendPQExpBuffer(&buf, " STATISTICS %s", PQgetvalue(result, i, 9));

Finally, the patch might be more complete if a note is added in the documentation.
Have you considered adding something in doc/src/sgml/ref/psql-ref.sgml? If no, will you consider it? If yes, why did you discard it?

Regards,
Georgios