go-tarantool icon indicating copy to clipboard operation
go-tarantool copied to clipboard

[MP_EXT] Extension encoder and decoder functions are not exportable

Open rybakit opened this issue 11 months ago • 2 comments

Consider making the current MessagePack extension encoder and decoder functions exportable. I’m personally interested in the decimal extension, where I would like to register my own extension but still be able to reuse the encoding logic from the tarantool/decimal package. The easiest way is to simply make the decimalEncoder() and decimalDecoder() functions start with a capital letter. Another option is to make them adhere to the MarshalerUnmarshaler interface from vmihailenco/msgpack/v5 and rename them to MarshalMsgpack() and UnmarshalMsgpack(), respectively.

rybakit avatar Dec 16 '24 15:12 rybakit

@oleg-jukovec, Looking deeper into the decimal extension implementation, it’s not entirely clear why the connector uses a wrapper for the decimal.Decimal type from shopspring/decimal. This made sense in the earlier implementation, where the wrapper provided additional methods like MarshalMsgpack() and UnmarshalMsgpack(). However, after refactoring the code to use msgpack v5, those methods are no longer present, and the wrapper doesn’t appear to serve any useful purpose.

To give some context to the problem I’m trying to solve: I want to register an encoder and decoder directly on the decimal.Decimal type without needing to wrap decimals in the connector's Decimal struct every time.

As an alternative to making decoders and encoders exportable, what do you think about removing the wrapper altogether? This solution would address my use case and align with the approach used for the UUID extension, which doesn’t rely on a wrapper and allows direct encoding and decoding of UUID values in the msgpack extension.

rybakit avatar Dec 25 '24 23:12 rybakit

I don't like the idea to use a types from external libraries as is into the connector: they may be incompatibility in the future or we can find a better type.

If the library declares its own types with transform methods, then in the future we can add methods with a new types or declare the old methods deprecated.

That is, it is not entirely convenient for the user of the library code, but it is good for the library and its development in the future, backward compatibility etc.

The uuid is an exception because we don't have much time to rewrite it (and it's probably not the best idea to rewrite something that works).

oleg-jukovec avatar Dec 29 '24 00:12 oleg-jukovec