Re: [Tarantool-patches] [PATCH v6 1/5] build, lua: built-in module datetime

[Safin Timur via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

On 19.08.2021 12:43, Serge Petrenko wrote:
> 
> 
> 19.08.2021 05:56, Timur Safin пишет:
> ...
>> diff --git a/src/lib/core/datetime.h b/src/lib/core/datetime.h
>> new file mode 100644
>> index 000000000..71feefded
>> --- /dev/null
>> +++ b/src/lib/core/datetime.h
>> @@ -0,0 +1,86 @@
>> +#pragma once
>> +/*
>> + * SPDX-License-Identifier: BSD-2-Clause
>> + *
>> + * Copyright 2021, Tarantool AUTHORS, please see AUTHORS file.
>> + */
>> +
>> +#include <stdint.h>
>> +#include <stdbool.h>
>> +#include "c-dt/dt.h"
>> +
>> +#if defined(__cplusplus)
>> +extern "C"
>> +{
>> +#endif /* defined(__cplusplus) */
>> +
>> +/**
>> + * We count dates since so called "Rata Die" date
>> + * January 1, 0001, Monday (as Day 1).
>> + * But datetime structure keeps seconds since
>> + * Unix "Epoch" date:
>> + * Unix, January 1, 1970, Thursday
>> + *
>> + * The difference between Epoch (1970-01-01)
>> + * and Rata Die (0001-01-01) is 719163 days.
>> + */
>> +
>> +#ifndef SECS_PER_DAY
>> +#define SECS_PER_DAY          86400
>> +#define DT_EPOCH_1970_OFFSET  719163
>> +#endif
>> +
>> +#define SECS_EPOCH_1970_OFFSET     \
>> +    ((int64_t)DT_EPOCH_1970_OFFSET * SECS_PER_DAY)
>> +/**
>> + * datetime structure keeps number of seconds since
>> + * Unix Epoch.
>> + * Time is normalized by UTC, so time-zone offset
>> + * is informative only.
>> + */
>> +struct datetime {
>> +    /** Seconds since Epoch. */
>> +    double secs;
>> +    /** Nanoseconds, if any. */
>> +    uint32_t nsec;
>> +    /** Offset in minutes from UTC. */
>> +    int32_t offset;
>> +};
>> +
>> +/**
>> + * Date/time interval structure
>> + */
>> +struct datetime_interval {
>> +    /** Relative seconds delta. */
>> +    double secs;
>> +    /** Nanoseconds delta, if any. */
>> +    uint32_t nsec;
>> +};
>> +
> 
> Hi! Thanks for working on this!
> 
> I don't see you use this struct anywhere in the patch.
> AFAICS interval operations are implemented via
> add_years(), add_months() and so on.
> 
> Let's drop the struct then. We may introduce it once
> it becomes necessary again.
> 
> Let's also drop the CTID_INTERVAL definition in lua/utils.c
> 
> LGTM, once you remove the interval.

Thanks for the spot - forgot to drop the C part. Here is incremental part.
------------------------------------------------
diff --git a/src/lib/core/datetime.h b/src/lib/core/datetime.h
index fc5717115..ab391a4f0 100644
--- a/src/lib/core/datetime.h
+++ b/src/lib/core/datetime.h
@@ -66,16 +66,6 @@ struct datetime {
  	int32_t offset;
  };

-/**
- * Date/time interval structure
- */
-struct datetime_interval {
-	/** Relative seconds delta. */
-	double secs;
-	/** Nanoseconds delta, if any. */
-	uint32_t nsec;
-};
-
  /**
   * Required size of datetime_to_string string buffer
   */
diff --git a/src/lua/utils.c b/src/lua/utils.c
index 2c89326f3..83e482b3d 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -49,7 +49,6 @@ uint32_t CTID_CHAR_PTR;
  uint32_t CTID_CONST_CHAR_PTR;
  uint32_t CTID_UUID;
  uint32_t CTID_DATETIME = 0;
-uint32_t CTID_INTERVAL = 0;


  void *
@@ -743,14 +742,6 @@ tarantool_lua_utils_init(struct lua_State *L)
  	(void) rc;
  	CTID_DATETIME = luaL_ctypeid(L, "struct datetime");
  	assert(CTID_DATETIME != 0);
-	rc = luaL_cdef(L, "struct datetime_interval {"
-			  "double secs;"
-			  "int32_t nsec;"
-			  "};");
-	assert(rc == 0);
-	(void) rc;
-	CTID_INTERVAL = luaL_ctypeid(L, "struct datetime_interval");
-	assert(CTID_INTERVAL != 0);

  	lua_pushcfunction(L, luaT_newthread_wrapper);
  	luaT_newthread_ref = luaL_ref(L, LUA_REGISTRYINDEX);
------------------------------------------------
> 
>> +/**
>> + * Convert datetime to string using default format
>> + * @param date source datetime value
>> + * @param buf output character buffer
>> + * @param len size ofoutput buffer
>> + */
>> +int
>> +datetime_to_string(const struct datetime *date, char *buf, int len);
>>
> ...
> 

Thanks,
Timur