[PATCH v2 6/8] decimal: allow to encode/decode decimals as MsgPack
Serge Petrenko
sergepetrenko at tarantool.org
Tue Aug 13 17:01:48 MSK 2019
> 13 авг. 2019 г., в 0:34, Konstantin Osipov <kostja at tarantool.org> написал(а):
>
> * Serge Petrenko <sergepetrenko at tarantool.org> [19/08/08 15:01]:
>> index d84748b71..89ea26b41 100644
>> --- a/src/lib/core/mp_user_types.h
>> +++ b/src/lib/core/mp_user_types.h
>> @@ -31,6 +31,12 @@
>> * SUCH DAMAGE.
>> */
>>
>> +#include "msgpuck.h"
>
> why this include?
For mp_type in msgpack_to_field_type() and field_type_to_msgpack().
>
>
> What is this header file standing (mp_user_tpyes.h) for?
>
> User types are enum field_type. What other user types do you have
> in mind? Perhaps you mean user-defined types? Well, decimal is not
> a user defined type, it's built-in. The fact that it has to use
> mp_ext format type in msgpack is just a coincidence of Tarantool
> using msgpack for its internal data representation.
Ok, we may name the file extension_types or ext_types or mp_ext_types.
>
>> +++ b/src/lib/core/mpstream.h
>> @@ -32,6 +32,7 @@
>> */
>>
>> #include "diag.h"
>> +#include "decimal.h"
>>
>
> why this include?
For decimal_t. Because
decimal_t is a typedef for decNumber, and decNumber is a typedef
for an anonymous struct.
I can patch decNumber so that we have a struct decNumber. Then we
will be able to get away with typedef struct decNumber decimal_t in
such headers.
>
> Please, please, C is not PHP, you don't include everything around
> just because it was a bad day.
>
>> index 6fc2b8278..2e6b5c163 100644
>> --- a/src/lua/msgpack.c
>> +++ b/src/lua/msgpack.c
>> @@ -41,6 +41,11 @@
>> #include <small/region.h>
>> #include <small/ibuf.h>
>>
>> +#include "lua/decimal.h"
>> +#include "lib/core/decimal.h"
>> +#include "lib/core/mp_decimal.h"
>> +#include "lib/core/mp_user_types.h"
>> +
>
> This is a dependency hell.
>
> Why did you add a separate header for decimal encoding/encoding if
> you have to include *everything* in here anyway? Noone will ever
> be able to understand why you decided to include all of these
> here.
Do you want me to add comments describing why we need each header?
>
>
>
> --
> Konstantin Osipov, Moscow, Russia
--
Serge Petrenko
sergepetrenko at tarantool.org
More information about the Tarantool-patches
mailing list