I noticed that logicalrep_typmap_gettypname's only caller is
slot_store_error_callback, which is an error callback. So by raising an
error from the former, we would effectively just hide the actual reason
for the error behind the error that the cache entry cannot be found.
Therefore, I'm inclined to make this function raise a warning, then
return a substitute value (something like "unrecognized type XYZ"). Do
the same for the case with the mismatched major versions. Lastly, if
LogicalRepTypMap is not defined, also throw a warning and return a
substitute value rather than try to initialize the hash table:
initializing the table is not going to make the entry show up in it,
anyway, so it's pretty pointless; and there's plenty chance for things
to go wrong, which is not a good idea in an error callback.
Do we need a new TAP test for this? Judging by
https://coverage.postgresql.org/src/backend/replication/logical/relation.c.gcov.html
showing all zeroes for the last function, we do. Same with
slot_store_error_callback in
https://coverage.postgresql.org/src/backend/replication/logical/worker.c.gcov.html
Thanks,
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
