* Fwd: Re: [PATCH v3 4/4] decimal: add MessagePack encoding/decoding support
@ 2019-06-06 9:27 Vladimir Davydov
2019-06-11 16:04 ` [tarantool-patches] " Serge Petrenko
0 siblings, 1 reply; 2+ messages in thread
From: Vladimir Davydov @ 2019-06-06 9:27 UTC (permalink / raw)
To: Serge Petrenko; +Cc: georgy, tarantool-patches, kostja
[ RESEND ]
On Tue, Jun 04, 2019 at 07:04:19PM +0300, Serge Petrenko wrote:
> Add methods to encode/decode decimals as msgpack.
>
> Part of #692
> ---
> cmake/BuildDecNumber.cmake | 1 +
> src/lib/core/decimal.c | 64 +++++++++++
> src/lib/core/decimal.h | 38 +++++++
> src/lib/core/mp_user_types.h | 38 +++++++
> test/unit/decimal.c | 73 ++++++++++++-
> test/unit/decimal.result | 204 ++++++++++++++++++++++++++++++++++-
> 6 files changed, 416 insertions(+), 2 deletions(-)
> create mode 100644 src/lib/core/mp_user_types.h
>
> diff --git a/cmake/BuildDecNumber.cmake b/cmake/BuildDecNumber.cmake
> index 80942fe05..abc6c64c4 100644
> --- a/cmake/BuildDecNumber.cmake
> +++ b/cmake/BuildDecNumber.cmake
> @@ -4,6 +4,7 @@ macro(decnumber_build)
> set(decnumber_src
> ${PROJECT_SOURCE_DIR}/third_party/decNumber/decNumber.c
> ${PROJECT_SOURCE_DIR}/third_party/decNumber/decContext.c
> + ${PROJECT_SOURCE_DIR}/third_party/decNumber/decPacked.c
> )
>
> add_library(decNumber STATIC ${decnumber_src})
> diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
> index 12250da95..7b3b1d8c9 100644
> --- a/src/lib/core/decimal.c
> +++ b/src/lib/core/decimal.c
> @@ -30,7 +30,10 @@
> */
>
> #include "decimal.h"
> +#include "mp_user_types.h"
> +#include "src/lib/msgpuck/msgpuck.h"
> #include "third_party/decNumber/decContext.h"
> +#include "third_party/decNumber/decPacked.h"
> #include <stdlib.h>
> #include <assert.h>
>
> @@ -291,3 +294,64 @@ decimal_sqrt(decimal *res, const decimal *lhs, uint32_t precision)
>
> return decimal_finalize(res, &op_context);
> }
> +
> +static inline uint32_t
> +decimal_len(decimal *dec)
> +{
> + /* 1 + ceil((digits + 1) / 2) */
Bad indentation.
> + return 2 + dec->digits / 2;
> +}
> +
> +uint32_t
> +mp_sizeof_decimal(decimal *dec) {
> + uint32_t l = decimal_len(dec);
> + return mp_sizeof_extl(l) + l;
Better use mp_sizeof_ext instead of mp_sizeof_extl here?
> +}
> +
> +char *
> +mp_encode_decimal(char *data, decimal *dec)
> +{
> + uint32_t len = decimal_len(dec);
> + data = mp_encode_extl(data, MP_DECIMAL, len);
> + data = mp_store_u8(data, decimal_scale(dec));
> + len--;
> + int32_t scale;
> + char *tmp = (char *)decPackedFromNumber((uint8_t *)data, len, &scale, dec);
> + assert(tmp == data);
> + assert(scale == (int32_t)decimal_scale(dec));
> + data += len;
> + return data;
> +}
> +
> +decimal *
> +mp_decode_decimal_raw(const char **data, decimal *dec, uint32_t len)
> +{
> + int32_t scale = mp_load_u8(data);
> + len--;
> + decimal *res = decPackedToNumber((uint8_t *)*data, len, &scale, dec);
> + if (res)
> + *data += len;
> + else
> + (*data)--;
> + return res;
> +}
> +
> +decimal *
> +mp_decode_decimal(const char **data, decimal *dec)
> +{
> + int8_t type;
> + uint32_t len;
> +
> + if (mp_typeof(**data) != MP_EXT)
> + return NULL;
> + const char *const svp = *data;
> + len = mp_decode_extl(data, &type);
> +
> + if (type != MP_DECIMAL || len == 0)
> + return NULL;
> + decimal *res = mp_decode_decimal_raw(data, dec, len);
> + if (!res) {
> + *data = svp;
> + }
> + return res;
> +}
> diff --git a/src/lib/core/decimal.h b/src/lib/core/decimal.h
> index f2812f500..cfcc98073 100644
> --- a/src/lib/core/decimal.h
> +++ b/src/lib/core/decimal.h
> @@ -156,4 +156,42 @@ decimal_exp(decimal *res, const decimal *lhs, uint32_t precision);
> decimal *
> decimal_sqrt(decimal *res, const decimal *lhs, uint32_t precision);
>
> +
> +/**
> + * Return length of message pack encoded decimal with extension header.
> + * Equivalent to mp_sizeof_ext(decimal representation length)
> + */
> +uint32_t
> +mp_sizeof_decimal(decimal *dec);
> +
> +/**
> + * Encode a decimal value to msgpack.
> + *
> + * @return data + mp_sizeof_decimal(dec)
> + */
> +char *
> +mp_encode_decimal(char *data, decimal *dec);
> +
> +/**
> + * Decode a decimal value as though msgpack
> + * ext header was already decoded.
> + *
> + * \post *data = *data + value representation length
> + * @return NULL if value encoding is incorrect
> + * dec otherwise.
> + */
> +decimal *
> +mp_decode_decimal_raw(const char **data, decimal *dec, uint32_t len);
> +
> +/**
> + * Decode a decimal encoded as msgpack with ext header.
> + *
> + * \post *data = *data + mp_sizeof_decimal(dec)
> + * @return NULL if mp_typeof(**data) != MP_EXT
> + * or value encoding is incorrect
> + * dec otherwise.
> + */
> +decimal *
> +mp_decode_decimal(const char **data, decimal *dec);
> +
Mixing mp_ and decimal_ methods in the same file looks ugly, because
msgpack is just one format - we might add other formats in future and it
doesn't sound like a good idea to implement their decoders in decimal.c.
IMO decimal.h should only contain methods to pack/unpack decimals in
some format, say
decimal_pack_len/decimal_pack/decimal_unpack
or
decimal_encode/decimal_decode
mp_ methods should live in some other file (mp_ext, mp_ext_decimal,
mp_decimal.[hc]?)
And I think it's better add and test those generic methods in patch 2,
along with the decimal infrastructure while this patch should be about
msgpack ext only.
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [tarantool-patches] [PATCH v3 4/4] decimal: add MessagePack encoding/decoding support
2019-06-06 9:27 Fwd: Re: [PATCH v3 4/4] decimal: add MessagePack encoding/decoding support Vladimir Davydov
@ 2019-06-11 16:04 ` Serge Petrenko
0 siblings, 0 replies; 2+ messages in thread
From: Serge Petrenko @ 2019-06-11 16:04 UTC (permalink / raw)
To: Vladimir Davydov; +Cc: Georgy Kirichenko, Konstantin Osipov, tarantool-patches
> 6 июня 2019 г., в 12:27, Vladimir Davydov <vdavydov.dev@gmail.com> написал(а):
>
> [ RESEND ]
>
> On Tue, Jun 04, 2019 at 07:04:19PM +0300, Serge Petrenko wrote:
>> Add methods to encode/decode decimals as msgpack.
>>
>> Part of #692
>> ---
>> cmake/BuildDecNumber.cmake | 1 +
>> src/lib/core/decimal.c | 64 +++++++++++
>> src/lib/core/decimal.h | 38 +++++++
>> src/lib/core/mp_user_types.h | 38 +++++++
>> test/unit/decimal.c | 73 ++++++++++++-
>> test/unit/decimal.result | 204 ++++++++++++++++++++++++++++++++++-
>> 6 files changed, 416 insertions(+), 2 deletions(-)
>> create mode 100644 src/lib/core/mp_user_types.h
>>
>> diff --git a/cmake/BuildDecNumber.cmake b/cmake/BuildDecNumber.cmake
>> index 80942fe05..abc6c64c4 100644
>> --- a/cmake/BuildDecNumber.cmake
>> +++ b/cmake/BuildDecNumber.cmake
>> @@ -4,6 +4,7 @@ macro(decnumber_build)
>> set(decnumber_src
>> ${PROJECT_SOURCE_DIR}/third_party/decNumber/decNumber.c
>> ${PROJECT_SOURCE_DIR}/third_party/decNumber/decContext.c
>> + ${PROJECT_SOURCE_DIR}/third_party/decNumber/decPacked.c
>> )
>>
>> add_library(decNumber STATIC ${decnumber_src})
>> diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
>> index 12250da95..7b3b1d8c9 100644
>> --- a/src/lib/core/decimal.c
>> +++ b/src/lib/core/decimal.c
>> @@ -30,7 +30,10 @@
>> */
>>
>> #include "decimal.h"
>> +#include "mp_user_types.h"
>> +#include "src/lib/msgpuck/msgpuck.h"
>> #include "third_party/decNumber/decContext.h"
>> +#include "third_party/decNumber/decPacked.h"
>> #include <stdlib.h>
>> #include <assert.h>
>>
>> @@ -291,3 +294,64 @@ decimal_sqrt(decimal *res, const decimal *lhs, uint32_t precision)
>>
>> return decimal_finalize(res, &op_context);
>> }
>> +
>> +static inline uint32_t
>> +decimal_len(decimal *dec)
>> +{
>> + /* 1 + ceil((digits + 1) / 2) */
>
> Bad indentation.
>
>> + return 2 + dec->digits / 2;
>> +}
>> +
>> +uint32_t
>> +mp_sizeof_decimal(decimal *dec) {
>> + uint32_t l = decimal_len(dec);
>> + return mp_sizeof_extl(l) + l;
>
> Better use mp_sizeof_ext instead of mp_sizeof_extl here?
>
>> +}
>> +
>> +char *
>> +mp_encode_decimal(char *data, decimal *dec)
>> +{
>> + uint32_t len = decimal_len(dec);
>> + data = mp_encode_extl(data, MP_DECIMAL, len);
>> + data = mp_store_u8(data, decimal_scale(dec));
>> + len--;
>> + int32_t scale;
>> + char *tmp = (char *)decPackedFromNumber((uint8_t *)data, len, &scale, dec);
>> + assert(tmp == data);
>> + assert(scale == (int32_t)decimal_scale(dec));
>> + data += len;
>> + return data;
>> +}
>> +
>> +decimal *
>> +mp_decode_decimal_raw(const char **data, decimal *dec, uint32_t len)
>> +{
>> + int32_t scale = mp_load_u8(data);
>> + len--;
>> + decimal *res = decPackedToNumber((uint8_t *)*data, len, &scale, dec);
>> + if (res)
>> + *data += len;
>> + else
>> + (*data)--;
>> + return res;
>> +}
>> +
>> +decimal *
>> +mp_decode_decimal(const char **data, decimal *dec)
>> +{
>> + int8_t type;
>> + uint32_t len;
>> +
>> + if (mp_typeof(**data) != MP_EXT)
>> + return NULL;
>> + const char *const svp = *data;
>> + len = mp_decode_extl(data, &type);
>> +
>> + if (type != MP_DECIMAL || len == 0)
>> + return NULL;
>> + decimal *res = mp_decode_decimal_raw(data, dec, len);
>> + if (!res) {
>> + *data = svp;
>> + }
>> + return res;
>> +}
>> diff --git a/src/lib/core/decimal.h b/src/lib/core/decimal.h
>> index f2812f500..cfcc98073 100644
>> --- a/src/lib/core/decimal.h
>> +++ b/src/lib/core/decimal.h
>> @@ -156,4 +156,42 @@ decimal_exp(decimal *res, const decimal *lhs, uint32_t precision);
>> decimal *
>> decimal_sqrt(decimal *res, const decimal *lhs, uint32_t precision);
>>
>> +
>> +/**
>> + * Return length of message pack encoded decimal with extension header.
>> + * Equivalent to mp_sizeof_ext(decimal representation length)
>> + */
>> +uint32_t
>> +mp_sizeof_decimal(decimal *dec);
>> +
>> +/**
>> + * Encode a decimal value to msgpack.
>> + *
>> + * @return data + mp_sizeof_decimal(dec)
>> + */
>> +char *
>> +mp_encode_decimal(char *data, decimal *dec);
>> +
>> +/**
>> + * Decode a decimal value as though msgpack
>> + * ext header was already decoded.
>> + *
>> + * \post *data = *data + value representation length
>> + * @return NULL if value encoding is incorrect
>> + * dec otherwise.
>> + */
>> +decimal *
>> +mp_decode_decimal_raw(const char **data, decimal *dec, uint32_t len);
>> +
>> +/**
>> + * Decode a decimal encoded as msgpack with ext header.
>> + *
>> + * \post *data = *data + mp_sizeof_decimal(dec)
>> + * @return NULL if mp_typeof(**data) != MP_EXT
>> + * or value encoding is incorrect
>> + * dec otherwise.
>> + */
>> +decimal *
>> +mp_decode_decimal(const char **data, decimal *dec);
>> +
>
> Mixing mp_ and decimal_ methods in the same file looks ugly, because
> msgpack is just one format - we might add other formats in future and it
> doesn't sound like a good idea to implement their decoders in decimal.c.
> IMO decimal.h should only contain methods to pack/unpack decimals in
> some format, say
>
> decimal_pack_len/decimal_pack/decimal_unpack
>
> or
>
> decimal_encode/decimal_decode
>
> mp_ methods should live in some other file (mp_ext, mp_ext_decimal,
> mp_decimal.[hc]?)
Will do after decimal to Lua exposure, no problem.
>
> And I think it's better add and test those generic methods in patch 2,
> along with the decimal infrastructure while this patch should be about
> msgpack ext only.
Added the methods to patch 2 and resent the patchset as v4.
--
Serge Petrenko
sergepetrenko@tarantool.org
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-06-11 16:04 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-06 9:27 Fwd: Re: [PATCH v3 4/4] decimal: add MessagePack encoding/decoding support Vladimir Davydov
2019-06-11 16:04 ` [tarantool-patches] " Serge Petrenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox