lib/ecto/adapters/myxql/connection.ex: refactored improper lists

[bradhanks]

refactored 10 lists

5 had a string literal as the tail
1 was a variable with a is_binary guard
4 were Integer.to_string functions

[greg-rychlewski]

Hi @bradhanks ,

Improper lists are 100% intentional. They save some bytes when sending the query string to the database.

[bradhanks]

Hi @bradhanks ,

Improper lists are 100% intentional. They save some bytes when sending the query string to the database.

oh nice. learned something new :)

[greg-rychlewski]

Sorry I mixed up the adapter behaviour with the driver behaviour (Postgrex, etc.).

In this case it's not about sending it to the database but rather this construct will be immediately passed to IO.iodata_to_binary to create the query string. And iodata can work with improper lists. So basically there is no need to wrap the value in a list.

[bradhanks]

Sorry I mixed up the adapter behaviour with the driver behaviour (Postgrex, etc.).

In this case it's not about sending it to the database but rather this construct will be immediately passed to IO.iodata_to_binary to create the query string. And iodata can work with improper lists. So basically there is no need to wrap the value in a list.

Just to solidify my own understanding, if you don't mind, are there issues with the below statement?

There's really no issue using a cons operator to add a string to the end of an iolist. The list isn't improper if all the elements are binaries.

[greg-rychlewski]

It looks like there is a slight misunderstanding in it.

An improper list isn't about whether the elements are binaries but whether the last element's tail is the empty list. Every proper list can be represented as [head | tail] where tail is another list. So for example, [1, 2] is actually [1 | [2 | []]]. If you built a similar improper list it would instead be just [1 | 2] (the last element is not a list).

Where binaries play a special role is in iolists. This is the definition from erlang:

iolist ::= []
        |   Binary
        |   [iohead | iolist]
        ;
iohead ::= Binary
        |   Byte (integer in the range [0..255])
        |   iolist
        ;

So when creating an improper list inside of an iolist, the last tail can be a binary but not an integer.

[bradhanks]

It looks like there is a slight misunderstanding in it.

An improper list isn't about whether the elements are binaries but whether the last element's tail is the empty list. Every proper list can be represented as [head | tail] where tail is another list. So for example, [1, 2] is actually [1 | [2 | []]]. If you built a similar improper list it would instead be just [1 | 2] (the last element is not a list).

Where binaries play a special role is in iolists. This is the definition from erlang:

iolist ::= []
        |   Binary
        |   [iohead | iolist]
        ;
iohead ::= Binary
        |   Byte (integer in the range [0..255])
        |   iolist
        ;

So when creating an improper list inside of an iolist, the last tail can be a binary but not an integer.

It looks like there is a slight misunderstanding in it.

An improper list isn't about whether the elements are binaries but whether the last element's tail is the empty list. Every proper list can be represented as [head | tail] where tail is another list. So for example, [1, 2] is actually [1 | [2 | []]]. If you built a similar improper list it would instead be just [1 | 2] (the last element is not a list).

Where binaries play a special role is in iolists. This is the definition from erlang:

iolist ::= []
        |   Binary
        |   [iohead | iolist]
        ;
iohead ::= Binary
        |   Byte (integer in the range [0..255])
        |   iolist
        ;

So when creating an improper list inside of an iolist, the last tail can be a binary but not an integer.

ok so an iolist can be improper or proper depending on if it ends in an empty list or binary.

[greg-rychlewski]

Yeah that is correct.

[bradhanks]

Yeah that is correct.
Thanks for taking the time to explain that to me! 👍