I'm not a fan of error messages like
relation "%s" is not a table, foreign table, or materialized view
It doesn't tell me what's wrong, it only tells me what else could have
worked. It's also tedious to maintain and the number of combinations
grows over time.
This was discussed many years ago in [0], with the same arguments, and
there appeared to have been general agreement to change this, but then
the thread stalled somehow on some technical details.
Attached is another attempt to improve this. I have rewritten the
primary error messages using the principle of "cannot do this with that"
and then added a detail message to show what relkind the object has.
For example:
-ERROR: relation "ti" is not a table, foreign table, or materialized view
+ERROR: cannot define statistics for relation "ti"
+DETAIL: "ti" is an index.
and
-ERROR: "test_foreign_table" is not a table, materialized view, or
TOAST table
+ERROR: relation "test_foreign_table" does not have a visibility map
+DETAIL: "test_foreign_table" is a foreign table.
You can see more instances of this in the test diffs in the attached patch.
In passing, I also changed a few places to use the RELKIND_HAS_STORAGE()
macro. This is related because it allows writing more helpful error
messages, such as in pgstatindex.c.
One question on a detail arose:
check_relation_relkind() in pg_visibility.c accepts RELKIND_RELATION,
RELKIND_MATVIEW, and RELKIND_TOASTVALUE, but pgstatapprox.c only accepts
RELKIND_RELATION and RELKIND_MATVIEW, even though they both look for a
visibility map. Is that an intentional omission? If so, it should be
commented better.
[0]:
https://www.postgresql.org/message-id/flat/AANLkTimR_sZ_wKd1cgqVG1PEvTvdr9j7zD%2B3_NPvfaa_%40mail.gmail.com
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
