[Hope it's OK if I move this thread to -hackers, as part of CF review.]
On Sat, Jun 9, 2012 at 2:40 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> Hi,
>
> I noticed this while testing 9.2, but it seems to go back to at least
> 8.3. Tab completion of function arguments doesn't work if the function
> is schema-qualified or double-quoted. So for example,
>
> DROP FUNCTION my_function ( <TAB>
>
> completes the functions arguments, but
>
> DROP FUNCTION my_schema.my_function ( <TAB>
>
> doesn't offer any completions, and nor does
>
> DROP FUNCTION "my function" ( <TAB>
+1 for the idea. I find the existing behavior rather confusing,
particularly the fact that a schema-qualified function name will be
tab-completed, i.e. this works.
DROP FUNCTION my_schema.my<TAB>
but then, as your second example above shows, no completions are
subsequently offered for the function arguments.
As a side note unrelated to this patch, I also dislike how function
name tab-completions will not fill in the opening parenthesis, which
makes for unnecessary work for the user, as one then has to type the
parenthesis and hit tab again to get possible completions for the
function arguments. The current behavior causes:
DROP FUNCTION my_f<TAB>
which completes to:
DROP FUNCTION my_function
enter parenthesis, and hit tab:
DROP FUNCTION my_function(<TAB>
which, if there is only one match, could complete to:
DROP FUNCTION my_function(integer)
when the last three steps could have been consolidated with better
tab-completion. Perhaps this could be a TODO.
> The attached patch fixes these problems by introducing a new macro
> COMPLETE_WITH_ARG, similar to the existing COMPLETE_WITH_ATTR, which
> seems to be the nearest analogous code that covers all the edge cases.
Anyway, on to the review of the patch itself:
* Submission *
Patch applies cleanly to git head, and regression tests are not
expected for tab-completion enhancements.
* Features & Usability *
I've verified that tab-completing of the first argument to functions
for DROP FUNCTION and ALTER FUNCTION commands for the most part works
as expected. The one catch I noticed was that
Query_for_list_of_arguments wasn't restricting its results to
currently-visible functions, so with a default search_path, if you
have these two functions defined:
public.doppelganger(text)
my_schema.doppelganger(bytea)
and then try:
DROP FUNCTION doppelganger(<TAB>
you get tab-completions for both "text)" and "bytea(", when you
probably expected only the former. That's easy to fix though, please
see attached v2 patch.
* Coding *
The new macro COMPLETE_WITH_ARG seems fine. The existing code used
malloc() directly for DROP FUNCTION and ALTER FUNCTION
(tab-complete.c, around lines 867 and 2190), which AIUI is frowned
upon in favor of pg_malloc(). The patch avoids this ugliness by using
the new COMPLETE_WITH_ARG macro, so that's a nice fixup.
Overall, a nice fix for an overlooked piece of the tab-completion machinery.
Josh
