klarna/erlavro

Encoder / Decoder - ability to translate null -> nil for Elixir clients

davydog187 opened this issue · 12 comments

Thanks for the great project!

I am interested in implementing this myself, but I thought I'd raise it as an issue if someone got to it first. For Elixir clients of erlavro, nil needs to be swapped with :null on the encoding side, and on the decoding side, :null should be swapped with nil.

Since this might not be desirable for all consumers, it seems that this could just be an option passed to :avro.make_simple_encoder/2 and :avro.make_simple_decoder/2

I'm not sure if we really need an encoder option to support nil as :null encoding.
I believe it's just an extra function clause for avro_primitive.cast/2.

To support :null to nil translation in decoded result,
we'll need to add a new option to decoder_options() type.

Both JSON and binary decoders should be updated.

I have not coded much elixir, out of curiosity, is it a big pain having to deal with :null ?

Thanks for the response @zmstone. Elixir uses the atom :nil to represent null values, and has syntactic sugar for it so that it can be written as nil in Elixir. true and false are the same. See the Elixir builit-in types.

  1. Can the elixir caller convert :nil to :null ?
  2. Or to have a preprocessor macro to wrap the :nil matching clauses?

input/output can be a deep data structure, having to convert data can work but an unnecessary overhead.

I like the macro idea, but slightly different approach:
if we introduce a macro like this

-ifndef(ERLAVRO_NULL_VALUE).
-define(ERLAVRO_NULL_VALUE, null).
-end.

then users can override it at compile time.

can be problematic for large projects sharing this dependency,
and a part of the project is using :null already.

mikpe commented

Do I understand this issue correctly in that it would result in a message being decoded into different terms depending on whether the caller is Erlang or Elixir? I think that's a bad idea.

I mean we can define null value at compile time, does not matter if the caller is Erlang or Elixir
we provide a default, the override is to be defined at user's project level.

mikpe commented

Make it a runtime option to the decoder.

The Elixir caller can convert nil to null, which is the work around were using right now in encoding. The issue as stated by @zmstone is that this requires doing a full tree traversal on the data structure, which is unnecessarily expensive.

As for decoding, a runtime option seems reasonable. Is that something that could be achieved by decoding hooks as well? I would avoid the decoding hooks solution though, as it introduces inconsistent behavior on the encoding/decoding boundaries.

mikpe commented

After a quick glance over the decoding code, it seems a hook could solve the problem without a code change.

On second thought, that’s probably a reasonable compromise. So only my encoding PR should be necessary #92

Hi guys, could you provide an example of decoder hook for this issue?
ps: Since I'm not the first to run into this issue, I think it would be nice to mention about it in docs.

@SerikDM you can check out how AvroSchema uses it

https://github.com/cogini/avro_schema/blob/master/lib/avro_schema.ex#L242

Alternatively, if you're using Elixir, you may want to try my other library, AvroEx