Re: NLS handling fixes.

Michael Paquier <michael(at)paquier(dot)xyz> writes:
> I have been looking at all the things you are proposing here, and it
> seems to me that you are right for these. I lack a bit of knowledge
> about the translation of items, but can such things be back-patched?

I would certainly *not* back-patch the GetConfigOptionByNum change,
as that will be a user-visible behavioral change that people will
not be expecting. We might get away with back-patching the other changes,
but SHOW ALL output seems like something that people might be expecting
not to change drastically in a minor release.

Some general review notes:

* I believe our usual convention is to write _() not gettext().
This patch set is pretty schizophrenic about that.

* In the patch fixing view_query_is_auto_updatable misuse, nothing's
been done to remove the underlying cause of the errors, which IMO
is that the header comment for view_query_is_auto_updatable fails to
specify the return convention. I'd consider adding a comment along
the lines of

* view_query_is_auto_updatable - test whether the specified view definition
* represents an auto-updatable view. Returns NULL (if the view can be updated)
* or a message string giving the reason that it cannot be.
+*
+* The returned string has not been translated; if it is shown as an error
+* message, the caller should apply _() to translate it.
*

* Perhaps pg_GSS_error likewise could use a comment saying the error
string must be translated already. Or we could leave its callers alone
and put the _() into it, but either way the API contract ought to be
written down.

* The proposed patch for psql/common.c seems completely wrong, or at
least it has a lot of side-effects other than enabling header translation,
and I doubt they are desirable.

regards, tom lane