Conversation
Fixes dbcli#1204. fields() guarded against a missing arg_modes, but zipped arg_names/arg_types/arg_modes unconditionally otherwise. A function with a truthy arg_modes but no arg_names (e.g. an unnamed variadic parameter, as in the reported hstore/variadic example) hit 'NoneType' object is not iterable instead of being handled. Fall back to None placeholders for arg_names/arg_types the same way args() already falls back for arg_modes a few lines up.
Written in June on a branch that never reached main, and recovered now because upstream dbcli#1638 touches the same lines. fields() zipped arg_names with arg_modes and bailed out when the names were missing, so a function like labels(variadic text[]) returns hstore offered no output column at all: "select labels from labels(...)" had nothing to complete. The same happens for a set-returning function declared with TABLE(...) and no argument names. When no argument carries an output mode, the function name is used as the column name, which is exactly what the branch above already does for functions declared without output parameters. Guarding the zip also removes the "'NoneType' object is not iterable" crash that upstream still has; our fork returned an empty list instead of crashing, so the symptom here was a silently missing completion rather than a traceback. The version bump and changelog entry from the original commit were left out: the version stays where it is and the entry is written for the current Upcoming section. 5 tests, 3 of which fail without the change.
|
Thanks for tracking this down. I hit the same crash in my fork a while back, so here is what I measured against your branch, in case it is useful. The fix removes the The reason is that f = FunctionMetadata("s", "f", None, ["int4", "text"], ["t", "t"], "record",
False, False, True, False, None)
f.fields() # [(None, 'int4'), (None, 'text')]and What worked for me was to treat "no argument carries an output mode, or the names are missing" the same way the branch a few lines above already treats a function declared without output parameters: use the function name as the column name. fields = [
ColumnMetadata(name, typ, [])
for name, typ, mode in zip(self.arg_names or [], self.arg_types or [], self.arg_modes)
if mode in ("o", "b", "t")
]
return fields or [ColumnMetadata(self.func_name, self.return_type, [])]That gives Either way the crash is worth fixing, so this is a suggestion on top of your change rather than an objection to it. |
DiegoDAF pointed out that the earlier change only moved the crash: for a TABLE(...) function declared without argument names, fields() now returned ColumnMetadata entries with name=None, and generate_alias() then failed iterating that None. When arg_modes has no usable output column (unnamed TABLE columns, or only a variadic parameter), fall back to the function name as the column name, like the branch above already does for functions without output parameters. The variadic case now completes 'labels' instead of offering nothing, and the TABLE case no longer reaches generate_alias() with None.
|
Thanks @DiegoDAF, you were right: with |
Fixes #1204.
Bug
Autocompleting a call to a table-returning or variadic function whose parameters have no names (e.g. an unnamed
variadic text[]argument, as in the reportedhstore/labels(variadic text[])example) crashes the completion thread:fields()only guards against a missingarg_modes(elif not self.arg_modes: ...), then zipsarg_names/arg_types/arg_modesunconditionally.arg_modesbeing truthy doesn't guaranteearg_namesis populated too — a function can have modes but no names, in which casearg_namesstaysNoneand thezip()raises before it ever gets to filter by mode.Fix
Fall back to
Noneplaceholders forarg_names/arg_typesthe same wayargs()(a few lines up in the same class) already falls back forarg_modes, so the zip always gets iterables.Test plan
test_function_metadata_fields_with_variadic_and_no_arg_names, directly reproducing the reported case viaFunctionMetadata.test_function_metadata_fields_table_mode_with_no_arg_names, checking a TABLE-mode function with unnamed columns returns sensibleColumnMetadata(not just an empty list).TypeErroragainst the unpatched code.pytest tests/ --ignore=tests/features— 2575 passed, 1 pre-existing failure unrelated to this change (a Windows-only tempfile permission issue in an unrelated alias-map test), 1 xfailed, 1 xpassed.ruff check/ruff formatclean.