=?utf-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org> writes:
> First, as noted in the test, it doesn't preserve the case of the input
> for keywords appended to the query result. This is easily fixed by
> using `pg_strdup_keyword_case()`, per the first attached patch.
I thought about that, and intentionally didn't do it, because it
would also affect the menus produced by tab completion. Currently,
keywords are (usually) visually distinct from non-keywords in those
menus, thanks to being upper-case where the object names usually
aren't:
regression=# create table foo (c1 int, c2 int);
CREATE TABLE
regression=# alter table foo rename c<TAB>
c1 c2 COLUMN CONSTRAINT
With this change, the keywords would be visually indistinguishable
from the object names, which I felt wouldn't be a net improvement.
We could do something hacky like matching case only when there's
no longer any matching object names, but that might be too magic.
> The second might be more of a matter of style or opinion, but I noticed
> a bunch of `if (foo) free(foo);`, which is redundant given that
> `free(NULL)` is a no-op. To simplify the code further, I also made
> `escape_string(NULL)` be a no-op, returning `NULL`.
Yeah. Our fairly longstanding convention is to avoid doing
free(NULL), dating back to when some platforms would crash on it.
I realize that's archaic now, but I'm not inclined to change
it in just some places.
regards, tom lane