Jump to conversation
Unresolved conversations (5)
@fcristovao fcristovao Jan 8, 2021
nittest of nits: consider optimizing the `lists:zip` away by recursively calling a function and matching on the heads of Values and Types to produce these.
Outdated
src/epgsql_wire.erl
seriyps
@fcristovao fcristovao Jan 8, 2021
nit: `ninstead` -> `instead`
Outdated
src/epgsql_wire.erl
@fcristovao fcristovao Jan 8, 2021
nit: consider turning this into a simple "translator" function. Then you can also use it for `init_copy_state(someFun(BinOrText), ...`. Notice the last function clause on `init_copy_state/4` will actually error out with `{format_mismatch, 0, binary}` instead of `{format_mismatch, text, binary}`.
Outdated
src/commands/epgsql_cmd_copy_from_stdin.erl
@fcristovao fcristovao Jan 8, 2021
This type doesn't match the response in `handle_message(?COPY_IN_RESPONSE, ... `. It should be `{ok, list(text | binary)}` or maybe something even stricter. As I was looking through the tests though, I also wondered, what is the point of returning a list of `text | binary`? It's confusing from a API's standpoint. Since the values will always have to be the same, I think it would be best to return just `text | binary`, instead of a list of them. Additionally, I think it would also be good to return a `csv` case, as they are distinct formats.
Outdated
src/commands/epgsql_cmd_copy_from_stdin.erl
seriyps
@fcristovao fcristovao Jan 8, 2021
This seems like it's never a possibility from `epgsql_wire:decode_complete`, nor it would fulfill the `response()` type above.
Outdated
src/commands/epgsql_cmd_copy_done.erl
seriyps
Resolved conversations (0)