Tom Lane <t...@sss.pgh.pa.us> writes:
> "tanghy.f...@fujitsu.com" <tanghy.f...@fujitsu.com> writes:
>> Thanks for your V16 patch, I tested it.
>> The results LGTM.
>
> Pushed, thanks for looking.
I wasn't following this thread, but I noticed a few small potential
improvements when I saw the commit.
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.
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`.
- ilmari
>From 7b278ea5c48d386e1c6f277f3d94520505205cd2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>
Date: Mon, 31 Jan 2022 00:42:00 +0000
Subject: [PATCH 1/2] Respect COMP_KEYWORD_CASE for additional keywords
When tacking on keywords to the result of a completion query, use
pg_strdup_keyword_case().
---
src/bin/psql/t/010_tab_completion.pl | 26 ++++++++++++++++++++------
src/bin/psql/tab-complete.c | 4 ++--
2 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.pl
index c4f6552ac9..c4a1d5e1bc 100644
--- a/src/bin/psql/t/010_tab_completion.pl
+++ b/src/bin/psql/t/010_tab_completion.pl
@@ -317,12 +317,26 @@ sub clear_line
clear_line();
-# check completion of a keyword offered in addition to object names
-# (that code path currently doesn't preserve case of what's typed)
-check_completion(
- "comment on constraint foo on dom\t",
- qr|DOMAIN|,
- "offer keyword in addition to query result");
+foreach (
+ ['lower', 'DOM', 'domain'],
+ ['upper', 'dom', 'DOMAIN'],
+ ['preserve-lower', 'dom', 'domain'],
+ ['preserve-upper', 'DOM', 'DOMAIN'],
+) {
+ my ($case, $in, $out) = @$_;
+
+ # check completion of a keyword offered in addition to object names
+ # and obeys COMP_KEYWORD_CASE
+ check_completion(
+ "\\set COMP_KEYWORD_CASE $case\n",
+ qr/postgres=#/,
+ "set completion case to '$case'");
+ check_completion(
+ "comment on constraint foo on $in\t",
+ qr|$out|,
+ "offer keyword in addition to query result (COMP_KEYWORD_CASE=$case)");
+ clear_line();
+}
clear_query();
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index b2ec50b4f2..0c7a99be0a 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -5032,7 +5032,7 @@ _complete_from_query(const char *simple_query,
if (pg_strncasecmp(text, item, strlen(text)) == 0)
{
num_other++;
- return pg_strdup(item);
+ return pg_strdup_keyword_case(item, text);
}
}
}
@@ -5050,7 +5050,7 @@ _complete_from_query(const char *simple_query,
if (pg_strncasecmp(text, item, strlen(text)) == 0)
{
num_other++;
- return pg_strdup(item);
+ return pg_strdup_keyword_case(item, text);
}
}
}
--
2.30.2
>From f4d38e9a819fa8650bea201784511654ec1d858e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>
Date: Mon, 31 Jan 2022 00:09:14 +0000
Subject: [PATCH 2/2] psql tab-complete: remove unnecessary null checks
free(NULL) is specified to be a no-op, so no need to check before
calling it. Also make escape_string(NULL) a no-op to match.
---
src/bin/psql/tab-complete.c | 37 ++++++++++++-------------------------
1 file changed, 12 insertions(+), 25 deletions(-)
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 0c7a99be0a..5115f02934 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -4555,11 +4555,9 @@ psql_completion(const char *text, int start, int end)
free(previous_words);
free(words_buffer);
free(text_copy);
- if (completion_ref_object)
- free(completion_ref_object);
+ free(completion_ref_object);
completion_ref_object = NULL;
- if (completion_ref_schema)
- free(completion_ref_schema);
+ free(completion_ref_schema);
completion_ref_schema = NULL;
/* Return our Grand List O' Matches */
@@ -4790,21 +4788,9 @@ _complete_from_query(const char *simple_query,
* up suitably-escaped copies of all the strings we need.
*/
e_object_like = make_like_pattern(objectname);
-
- if (schemaname)
- e_schemaname = escape_string(schemaname);
- else
- e_schemaname = NULL;
-
- if (completion_ref_object)
- e_ref_object = escape_string(completion_ref_object);
- else
- e_ref_object = NULL;
-
- if (completion_ref_schema)
- e_ref_schema = escape_string(completion_ref_schema);
- else
- e_ref_schema = NULL;
+ e_schemaname = escape_string(schemaname);
+ e_ref_object = escape_string(completion_ref_object);
+ e_ref_schema = escape_string(completion_ref_schema);
initPQExpBuffer(&query_buffer);
@@ -4959,12 +4945,9 @@ _complete_from_query(const char *simple_query,
/* Clean up */
termPQExpBuffer(&query_buffer);
free(e_object_like);
- if (e_schemaname)
- free(e_schemaname);
- if (e_ref_object)
- free(e_ref_object);
- if (e_ref_schema)
- free(e_ref_schema);
+ free(e_schemaname);
+ free(e_ref_object);
+ free(e_ref_schema);
}
/* Return the next result, if any, but not if the query failed */
@@ -5401,6 +5384,7 @@ pg_strdup_keyword_case(const char *s, const char *ref)
* escape_string - Escape argument for use as string literal.
*
* The returned value has to be freed.
+ * Returns NULL if called with NULL.
*/
static char *
escape_string(const char *text)
@@ -5408,6 +5392,9 @@ escape_string(const char *text)
size_t text_length;
char *result;
+ if (!text)
+ return text;
+
text_length = strlen(text);
result = pg_malloc(text_length * 2 + 1);
--
2.30.2
