Skip to content

Conversation

@tmr-g
Copy link

@tmr-g tmr-g commented Dec 3, 2025

hh.hh & hhmm.mm parsing in dt_iso_parse.c implemented.

Such formats are defined in ISO 8601-1:2019 (5.3.1.4).
They are used in tarantool datetime tests.

Need for tarantool/tarantool#12082 tarantool/tarantool#12083

@tmr-g tmr-g marked this pull request as ready for review December 3, 2025 22:16
tmr-g added a commit to tmr-g/tarantool that referenced this pull request Dec 4, 2025
- `datetime.c` `parse_tz_suffix` & it's clients refactored
  for better error detection & to simplify code:
  - offset value post condition check is moved to `parse_tz_suffix`.
  - this check uses tzoffset valid range instead of int16_t min/max.
  - offset arg type of `parse_tz_suffix` to valid for TZoffset int16_t.
- The better error detection lead to assertion fails of datetime tests
  with decimal fraction of hours and minutes. So we need
  tarantool/c-dt#3. It's the first way to fix the problem which
  is mentioned in tarantool#12082.
- datetime.c unit tests refactored. The number of tap test checks
  is being calculated using the lenght of test data tables.

FIXME: need to update c-dt submodule hash.

Requires tarantool/c-dt#3
Closes tarantool#12082

NO_DOC=FIXME, must have
NO_CHANGELOG=FIXME, must have
tmr-g added a commit to tmr-g/tarantool that referenced this pull request Dec 4, 2025
- `datetime.c` `parse_tz_suffix` & it's clients refactored
  for better error detection & to simplify code:
  - offset value post condition check is moved to `parse_tz_suffix`.
  - this check uses tzoffset valid range instead of int16_t min/max.
  - offset arg type of `parse_tz_suffix` fixed
    to valid for TZoffset int16_t.
- The better error detection lead to assertion fails of datetime tests
  with decimal fraction of hours and minutes. So we need
  tarantool/c-dt#3. It's the first way to fix the problem which
  is mentioned in tarantool#12082.
- datetime.c unit tests refactored. The number of tap test checks
  is being calculated using the lenght of test data tables.

FIXME: need to update c-dt submodule hash.

Requires tarantool/c-dt#3
Closes tarantool#12082

NO_DOC=FIXME, must have
NO_CHANGELOG=FIXME, must have
tmr-g added a commit to tmr-g/tarantool that referenced this pull request Dec 10, 2025
- `datetime.c` `parse_tz_suffix` & it's clients refactored
  for better error detection & to simplify code:
  - offset value post condition check is moved to `parse_tz_suffix`.
  - this check uses tzoffset valid range instead of int16_t min/max.
  - offset arg type of `parse_tz_suffix` fixed
    to valid for TZoffset int16_t.
- The better error detection lead to assertion fails of datetime tests
  with decimal fraction of hours and minutes. So we need
  tarantool/c-dt#3. It's the first way to fix the problem which
  is mentioned in tarantool#12082.
- datetime.c unit tests refactored to simpilfy it's support.
  The number of tap test checks is being calculated
  using the lenght of test data tables.

FIXME: need to update c-dt submodule hash.

Requires tarantool/c-dt#3
Closes tarantool#12082

NO_DOC=FIXME, must have
NO_CHANGELOG=FIXME, must have
tmr-g added a commit to tmr-g/tarantool that referenced this pull request Dec 10, 2025
- `datetime.c` `parse_tz_suffix` & it's clients refactored
  for better error detection & to simplify code:
  - offset value post condition check is moved to `parse_tz_suffix`.
  - this check uses tzoffset valid range instead of int16_t min/max.
  - offset arg type of `parse_tz_suffix` fixed
    to valid for TZoffset int16_t.
- The better error detection lead to assertion fails of datetime tests
  with decimal fraction of hours and minutes. So we need
  tarantool/c-dt#3. It's the first way to fix the problem which
  is mentioned in tarantool#12082.
- datetime.c unit tests refactored to simpilfy it's support.
  The number of tap test checks is being calculated
  using the lenght of test data tables.

FIXME: need to update c-dt submodule hash.

Requires tarantool/c-dt#3
Closes tarantool#12082

NO_DOC=FIXME, must have
NO_CHANGELOG=FIXME, must have
tmr-g added a commit to tmr-g/tarantool that referenced this pull request Dec 10, 2025
- `datetime.c` `parse_tz_suffix` & it's clients refactored
  for better error detection & to simplify code:
  - offset value post condition check is moved to `parse_tz_suffix`.
  - this check uses tzoffset valid range instead of int16_t min/max.
  - offset arg type of `parse_tz_suffix` fixed
    to valid for TZoffset int16_t.
- The better error detection lead to assertion fails of datetime tests
  with decimal fraction of hours and minutes. So we need
  tarantool/c-dt#3. It's the first way to fix the problem which
  is mentioned in tarantool#12082.

FIXME: need to update c-dt submodule hash.

Requires tarantool/c-dt#3
Closes tarantool#12082

NO_DOC=FIXME, must have
NO_CHANGELOG=FIXME, must have
@tmr-g tmr-g force-pushed the i12082 branch 2 times, most recently from 4a3058f to 91c68e5 Compare December 10, 2025 23:58
@sergos sergos requested a review from sergepetrenko December 11, 2025 08:42
@sergos sergos requested a review from ligurio December 11, 2025 08:46
Copy link

@sergepetrenko sergepetrenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch!
LGTM.

@sergepetrenko sergepetrenko assigned ligurio and tmr-g and unassigned sergepetrenko and ligurio Dec 12, 2025
tmr-g added a commit to tmr-g/tarantool that referenced this pull request Dec 16, 2025
- `datetime.c` `parse_tz_suffix` & it's clients refactored
  for better error detection & to simplify code:
  - offset value post condition check is moved to `parse_tz_suffix`.
  - this check uses tzoffset valid range instead of int16_t min/max.
  - offset arg type of `parse_tz_suffix` fixed
    to valid for TZoffset int16_t.
- The better error detection lead to assertion fails of datetime tests
  with decimal fraction of hours and minutes. So we need
  tarantool/c-dt#3. It's the first way to fix the problem which
  is mentioned in tarantool#12082.

FIXME: need to update c-dt submodule hash.

Requires tarantool/c-dt#3
Closes tarantool#12082

NO_DOC=FIXME, must have
NO_CHANGELOG=FIXME, must have
tmr-g added a commit to tmr-g/tarantool that referenced this pull request Dec 16, 2025
- `datetime.c` `parse_tz_suffix` & it's clients refactored
  for better error detection & to simplify code:
  - offset value post condition check is moved to `parse_tz_suffix`.
  - this check uses tzoffset valid range instead of int16_t min/max.
  - offset arg type of `parse_tz_suffix` fixed
    to valid for TZoffset int16_t.
- The better error detection lead to assertion fails of datetime tests
  with decimal fraction of hours and minutes. So we need
  tarantool/c-dt#3. It's the first way to fix the problem which
  is mentioned in tarantool#12082.

FIXME: need to update c-dt submodule hash.

Requires tarantool/c-dt#3
Closes tarantool#12082

NO_DOC=FIXME, must have
NO_CHANGELOG=FIXME, must have
tmr-g added a commit to tmr-g/tarantool that referenced this pull request Dec 16, 2025
- `datetime.c` `parse_tz_suffix` & it's clients refactored
  for better error detection & to simplify code:
  - offset value post condition check is moved to `parse_tz_suffix`.
  - this check uses tzoffset valid range instead of int16_t min/max.
  - offset arg type of `parse_tz_suffix` fixed
    to valid for TZoffset int16_t.
- The better error detection lead to assertion fails of datetime tests
  with decimal fraction of hours and minutes. So we need
  tarantool/c-dt#3. It's the first way to fix the problem which
  is mentioned in tarantool#12082.

FIXME: need to update c-dt submodule hash.

Requires tarantool/c-dt#3
Closes tarantool#12082

NO_DOC=FIXME, must have
NO_CHANGELOG=FIXME, must have
tmr-g added a commit to tmr-g/tarantool that referenced this pull request Dec 16, 2025
- `datetime.c` `parse_tz_suffix` & it's clients refactored
  for better error detection & to simplify code:
  - offset value post condition check is moved to `parse_tz_suffix`.
  - this check uses tzoffset valid range instead of int16_t min/max.
  - offset arg type of `parse_tz_suffix` fixed
    to valid for TZoffset int16_t.
- The better error detection lead to assertion fails of datetime tests
  with decimal fraction of hours and minutes. So we need
  tarantool/c-dt#3. It's the first way to fix the problem which
  is mentioned in tarantool#12082.

FIXME: need to update c-dt submodule hash.

Requires tarantool/c-dt#3
Closes tarantool#12082

NO_DOC=FIXME, must have
NO_CHANGELOG=FIXME, must have
tmr-g added a commit to tmr-g/tarantool that referenced this pull request Dec 16, 2025
- `datetime.c` `parse_tz_suffix` & it's clients refactored
  for better error detection & to simplify code:
  - offset value post condition check is moved to `parse_tz_suffix`.
  - this check uses tzoffset valid range instead of int16_t min/max.
  - offset arg type of `parse_tz_suffix` fixed
    to valid for TZoffset int16_t.
- The better error detection lead to assertion fails of datetime tests
  with decimal fraction of hours and minutes. So we need
  tarantool/c-dt#3. It's the first way to fix the problem which
  is mentioned in tarantool#12082.

FIXME: need to update c-dt submodule hash.

Requires tarantool/c-dt#3
Closes tarantool#12082

NO_DOC=FIXME, must have
NO_CHANGELOG=FIXME, must have
tmr-g added a commit to tmr-g/tarantool that referenced this pull request Dec 16, 2025
- `datetime.c` `parse_tz_suffix` & it's clients refactored
  for better error detection & to simplify code:
  - offset value post condition check is moved to `parse_tz_suffix`.
  - this check uses tzoffset valid range instead of int16_t min/max.
  - offset arg type of `parse_tz_suffix` fixed
    to valid for TZoffset int16_t.
- The better error detection lead to assertion fails of datetime tests
  with decimal fraction of hours and minutes. So we need
  tarantool/c-dt#3. It's the first way to fix the problem which
  is mentioned in tarantool#12082.

FIXME: UPDATE c-dt submodule hash TO COMMIT FROM tarantool/c-dt

Requires tarantool/c-dt#3
Closes tarantool#12082

NO_DOC=FIXME, must have
NO_CHANGELOG=FIXME, must have
tmr-g added a commit to tmr-g/tarantool that referenced this pull request Dec 16, 2025
- `datetime.c` `parse_tz_suffix` & it's clients refactored
  for better error detection & to simplify code:
  - offset value post condition check is moved to `parse_tz_suffix`.
  - this check uses tzoffset valid range instead of int16_t min/max.
  - offset arg type of `parse_tz_suffix` fixed
    to valid for TZoffset (int16_t).
- The better error detection lead to assertion fails of datetime tests
  with decimal fraction of hours and minutes. So we need
  tarantool/c-dt#3. It's the first way to fix the problem which
  is mentioned in tarantool#12082.

Requires tarantool/c-dt#3
Closes tarantool#12082

NO_DOC=bugfix

FIXME: UPDATE c-dt submodule hash TO COMMIT FROM tarantool/c-dt
tmr-g added a commit to tmr-g/tarantool that referenced this pull request Dec 16, 2025
- `datetime.c` `parse_tz_suffix` & it's clients refactored
  for better error detection & to simplify code:
  - offset value post condition check is moved to `parse_tz_suffix`.
  - this check uses tzoffset valid range instead of int16_t min/max.
  - offset arg type of `parse_tz_suffix` fixed
    to valid for TZoffset (int16_t).
- The better error detection lead to assertion fails of datetime tests
  with decimal fraction of hours and minutes. So we need
  tarantool/c-dt#3. It's the first way to fix the problem which
  is mentioned in tarantool#12082.

Requires tarantool/c-dt#3
Closes tarantool#12082

NO_DOC=bugfix

FIXME: UPDATE c-dt submodule hash TO COMMIT FROM tarantool/c-dt
@tmr-g tmr-g requested a review from ligurio December 16, 2025 19:41
tmr-g added a commit to tmr-g/tarantool that referenced this pull request Dec 16, 2025
- `datetime.c` `parse_tz_suffix` & it's clients refactored
  for better error detection & to simplify code:
  - offset value post condition check is moved to `parse_tz_suffix`.
  - this check uses tzoffset valid range instead of int16_t min/max.
  - offset arg type of `parse_tz_suffix` fixed
    to valid for TZoffset (int16_t).
- The better error detection lead to assertion fails of datetime tests
  with decimal fraction of hours and minutes. So we need
  tarantool/c-dt#3. It's the first way to fix the problem which
  is mentioned in tarantool#12082.

Requires tarantool/c-dt#3
Closes tarantool#12082

NO_DOC=bugfix

FIXME: UPDATE c-dt submodule hash TO COMMIT FROM tarantool/c-dt
Copy link
Member

@ligurio ligurio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch! Please see my comments.

dt_parse_iso.c Outdated
Comment on lines 108 to 116
* hh
* hh.fffffffff
* hh,fffffffff
* hhmm
* hhmm.fffffffff
* hhmm,fffffffff
* hhmmss
* hhmmss.fffffffff
* hhmmss,fffffffff
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please leave a comment with link to ISO 8601-1:2019, 5.3.1.4 where fractional part is described

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

dt_parse_iso.c Outdated
* hh,fffffffff
* hh:mm
* hh:mm.fffffffff
* hh:mm,fffffffff
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same as above: please leave a comment with link to standard

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 126 to 147
{ 1800, 0, "T00.5", 5 },
{ 450, 0, "T00.125", 7 },
{ 1199, 0, "T00.333333333", 13 },
{ 1800, 0, "T00,5", 5 },
{ 450, 0, "T00,125", 7 },
{ 1199, 0, "T00,333333333", 13 },
{ 30, 0, "T00:00.5", 8 },
{ 15, 0, "T00:00.25", 9 },
{ 7, 0, "T00:00.125", 10 },
{ 19, 0, "T00:00.333333333", 16 },
{ 30, 0, "T00:00,5", 8 },
{ 15, 0, "T00:00,25", 9 },
{ 7, 0, "T00:00,125", 10 },
{ 19, 0, "T00:00,333333333", 16 },
{ 30, 0, "T0000.5", 7 },
{ 15, 0, "T0000.25", 8 },
{ 7, 0, "T0000.125", 9 },
{ 19, 0, "T0000.333333333", 15 },
{ 30, 0, "T0000,5", 7 },
{ 15, 0, "T0000,25", 8 },
{ 7, 0, "T0000,125", 9 },
{ 19, 0, "T0000,333333333", 15 },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get how you chose a number for a fractional part.
I would check max (999999999), min (000000000) and average (any number in (0, 999999999)), and your choice: 5, 24, 125 and 333333333.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added min/max values.

Comment on lines 126 to 131
{ 1800, 0, "T00.5", 5 },
{ 450, 0, "T00.125", 7 },
{ 1199, 0, "T00.333333333", 13 },
{ 1800, 0, "T00,5", 5 },
{ 450, 0, "T00,125", 7 },
{ 1199, 0, "T00,333333333", 13 },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for unknown reasons .25 is missed for both formats

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

@@ -24,6 +24,7 @@
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please link to standard (ISO 8601-1:2019, 5.3.1.4) in the commit message

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

{ 1800, 0, "T00,5", 5 },
{ 450, 0, "T00,125", 7 },
{ 1199, 0, "T00,333333333", 13 },
{ 30, 0, "T00:00.5", 8 },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should these tests fail?

--- a/t/parse_iso_time.c
+++ b/t/parse_iso_time.c
@@ -125,10 +125,10 @@ const struct good_t {
     {     0,         0, "T000000.000000000",      17 },
     {  1800,         0, "T00.5",                  5 },
     {   450,         0, "T00.125",                7 },
-    {  1199,         0, "T00.333333333",         13 },
+    {  1199,         0, "T00.3333333331",         14 },
     {  1800,         0, "T00,5",                  5 },
     {   450,         0, "T00,125",                7 },
-    {  1199,         0, "T00,333333333",         13 },
+    {  1199,         0, "T00,3333333331",         14 },
     {    30,         0, "T00:00.5",               8 },
     {    15,         0, "T00:00.25",              9 },
     {     7,         0, "T00:00.125",            10 },

now tests passed

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More than 9 fraction digits is truncated. Comments added.

Here is the case with 10 digits (line 60):

c-dt/t/parse_iso_time.c

Lines 58 to 60 in edeaa5f

{ 0, 999999990, "T00:00:00.99999999", 18 },
{ 0, 999999999, "T00:00:00.999999999", 19 },
{ 0, 999999999, "T00:00:00.9999999999", 20 },

Similar tests added.

hh.hh & hhmm.mm parsing in dt_iso_parse.c implemented.
Such formats are defined in ISO 8601-1:2019 (5.3.1.4).
They are used in tarantool datetime tests.

Need for tarantool/tarantool#12082 tarantool/tarantool#12083
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants