`pgo:row/0` says `list() | map()` but a row is a tuple, breaking correct pattern matches under dialyzer

[Taure]

Since #127 landed on main, pgo:row/0 is declared as list() | map() and result() was tightened to rows := [row()]. A decoded row is never a list, so the wrong element type now propagates into every consumer through result(), and a correct pattern match on a default-decoded row is reported by dialyzer as unreachable.

Runtime behaviour is unaffected. This is purely a spec defect, but it is one that fails a downstream dialyzer gate.

The contradiction

src/pgo.erl:47:

pgo/src/pgo.erl

Line 47 in 36efee8

-type row() :: list() | map().

-type row() :: list() | map().

src/pgo_protocol.erl:576-595 says otherwise, in its own spec and in the code:

pgo/src/pgo_protocol.erl

Lines 576 to 595 in 36efee8

	-spec decode_row([#row_description_field{}], [binary()], atom(), proplists:proplist()) -> tuple().
	decode_row(Descs, Values, OIDMap, DecodeOptions) ->
	case proplists:get_bool(return_rows_as_maps, DecodeOptions) of
	false ->
	decode_row0(Descs, Values, OIDMap, DecodeOptions, []);
	true ->
	decode_row_as_map(Descs, Values, OIDMap, DecodeOptions, #{})
	end.
	decode_row_as_map([Desc=#row_description_field{name=Name} | DescsT], [Value | ValuesT], OIDMap, DecodeOptions, Acc) ->
	DecodedValue = decode_value(Desc, Value, OIDMap, DecodeOptions),
	decode_row_as_map(DescsT, ValuesT, OIDMap, DecodeOptions, Acc#{Name => DecodedValue});
	decode_row_as_map([], [], _OIDMap, _DecodeOptions, Acc) ->
	Acc.
	decode_row0([Desc | DescsT], [Value | ValuesT], OIDMap, DecodeOptions, Acc) ->
	DecodedValue = decode_value(Desc, Value, OIDMap, DecodeOptions),
	decode_row0(DescsT, ValuesT, OIDMap, DecodeOptions, [DecodedValue | Acc]);
	decode_row0([], [], _OIDMap, _DecodeOptions, Acc) ->
	list_to_tuple(lists:reverse(Acc)).

-spec decode_row([#row_description_field{}], [binary()], atom(), proplists:proplist()) -> tuple().
decode_row(Descs, Values, OIDMap, DecodeOptions) ->
    case proplists:get_bool(return_rows_as_maps, DecodeOptions) of
        false ->
            decode_row0(Descs, Values, OIDMap, DecodeOptions, []);
        true ->
            decode_row_as_map(Descs, Values, OIDMap, DecodeOptions, #{})
    end.
...
decode_row0([], [], _OIDMap, _DecodeOptions, Acc) ->
    list_to_tuple(lists:reverse(Acc)).

A row is a tuple by default and a map under return_rows_as_maps. No branch produces a list.

(#130 reports the complementary half of this, that decode_row/4's own spec says tuple() and omits the map case. This issue is about the list() in pgo:row/0, which is the part that escapes into user code via result().)

Reproduction

Two files against main, no database needed:

%% rebar.config
{erl_opts, [debug_info]}.
{deps, [{pgo, {git, "https://github.com/erleans/pgo.git", {branch, "main"}}}]}.

%% src/repro.erl
-module(repro).
-export([locked/1]).

locked(Pool) ->
    case pgo:query(~"SELECT pg_try_advisory_xact_lock($1)::text", [12345], #{pool => Pool}) of
        #{rows := [{~"true"}]} ->
            true;
        _ ->
            false
    end.

rebar3 dialyzer, OTP 29.0.2:

src/repro.erl
Line 6 Column 9: The pattern #{'rows':=[{<<116/utf8,114/utf8,117/utf8,101/utf8>>}]} can never match the type {'error',_} | #{'command':=atom(), 'num_rows':='table' | non_neg_integer(), 'rows':=[[any()] | map()]}

The match is correct. SELECT ...::text returns a single column, so at runtime the row really is {<<"true">>}. Dialyzer is also right to reject it against rows := [[any()] | map()], because a tuple is neither a list nor a map. There is no way to write this match correctly while the type says list().

It fires inside this repo too

rebar3 as test dialyzer on 36efee8 reports 135 warnings, 43 of which are this same pattern, spread across the CT suites (pgo_SUITE, pgo_geometric_SUITE and pgo_trace_SUITE among them):

test/pgo_SUITE.erl:112:46: The pattern
          #{'rows' := [{1}]} can never match the type
          {'error', _} |
          #{'command' := atom(),
            'num_rows' := 'table' | non_neg_integer(),
            'rows' := [[any()] | map()]}

Those assertions are correct. The suites match tuples because pgo returns tuples.

Fix

One line in src/pgo.erl:

-type row() :: tuple() | map().

With that, the 43 "can never match" warnings in this repo's own suites go to 0 (135 warnings down to 9, the remaining 9 unrelated and pre-existing), and the reproduction above dialyzes clean.

It pairs naturally with a one-line change at src/pgo_protocol.erl:576 so the decoder points at the canonical type instead of restating it, which also resolves #130:

-spec decode_row([#row_description_field{}], [binary()], atom(), proplists:proplist()) -> pgo:row().

One related decision: a caller-supplied decode_fun result becomes the row (src/pgo_handler.erl:544), and decode_fun/0 is typed fun((row(), fields()) -> row()). If returning arbitrary shapes from decode_fun is intended to be supported, that is better expressed by widening decode_fun/0's return type, since the built-in decoders never produce a list either way.

Aside: no_unknown is hiding a related class of defect

rebar.config:40:

{dialyzer, [{warnings, [no_unknown]}]}.

That suppresses reports of remote types which do not resolve, so a spec referring to a type that does not exist is silently never checked. Swapping no_unknown for unknown on 36efee8 surfaces these dangling references:

Reference	Site	Note
pgo:decode_opts/0	src/pgo_handler.erl:76, :409	pgo.erl exports decode_option/0, not decode_opts/0
pgo:conn/0	src/pgo_handler.erl:479	not defined in pgo.erl; pgo_pool:conn/0 exists
pg_pool:conn/0	src/pgo_connection.erl:53, src/pgo_protocol.erl:110	no pg_pool module in the tree; pgo_pool has conn/0
pgsql_error:pgsql_error_and_mention_field/0	src/pgo_internal.hrl:285, :292	no pgsql_error module in the tree; this is what #128 ran into
pgsql_error:pgsql_error_and_mention_field_type/0	src/pgo_protocol.erl:515	as above
maps:map/0	src/pgo_handler.erl:279, :294	should be map()

All pre-existing and harmless at runtime, but each one means the surrounding spec is not being checked at all. It also explains #128: that type genuinely does not resolve, and no_unknown is why the repo itself never notices. Given that #127 was about making these types trustworthy for downstream type checkers, turning unknown back on looks like it serves the same goal.

Happy to send a PR for the row/0 fix, with or without the decode_row/4 and no_unknown parts, whichever split suits you.