postgresql: Fix one-by-one error in db_to_date on negative dates
ClosedPublic
Actions

Authored by vlorentz on Dec 9 2021, 4:29 PM.

Details

Reviewers

ardumont
douardda

Group Reviewers

Reviewers

Commits

rDSTO34ca67e21f68: postgresql: Add tests for db_to_date.

Summary

Using int() on date.timestamp() rounded it up (toward zero),
but the semantics of model.Timestamp is that the actual time is
ts.seconds + ts.microseconds/1000000, so all negative dates were
shifted one second up.

In particular, this causes dates from
1969-12-31T23:59:59.000001 to 1969-12-31T23:59:59.999999
(inclusive) to smash into dates from
1970-01-01T00:00:00.000001 to 1970-01-01T00:00:00.999999,
which is how I discovered the issue.

Depends on D6814.

Diff Detail

Repository

rDSTO Storage manager

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 25599
Build 40030: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 40029: arc lint + arc unit

Event Timeline

vlorentz created this revision.Dec 9 2021, 4:29 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptDec 9 2021, 4:29 PM

Build is green

Patch application report for D6815 (id=24694)

Could not rebase; Attempt merge onto 7cb4128e40...

Updating 7cb4128e..110e2c3c
Fast-forward
 swh/storage/postgresql/converters.py            |   5 +-
 swh/storage/tests/test_postgresql_converters.py | 187 +++++++++++++++++-------
 2 files changed, 140 insertions(+), 52 deletions(-)

Changes applied before test

commit 110e2c3cbd6fca00a0e8cce5e43cf81c7fd216cc
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Dec 9 16:25:20 2021 +0100

    postgresql: Fix one-by-one error in db_to_date on negative dates
    
    Using `int()` on `date.timestamp()` rounded it up (toward zero),
    but the semantics of `model.Timestamp` is that the actual time is
    `ts.seconds + ts.microseconds/1000000`, so all negative dates were
    shifted one second up.
    
    In particular, this causes dates from
    `1969-12-31T23:59:59.000001` to `1969-12-31T23:59:59.999999`
    (inclusive) to smash into dates from
    `1970-01-01T00:00:00.000001` to `1970-01-01T00:00:00.999999`,
    which is how I discovered the issue.

commit 34ca67e21f68de920f603223f629070683018c2a
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Dec 9 16:23:44 2021 +0100

    postgresql: Add tests for db_to_date.

See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/1497/ for more details.

Harbormaster completed remote builds in B25547: Diff 24694.Dec 9 2021, 4:37 PM

vlorentz requested review of this revision.Dec 9 2021, 4:37 PM

vlorentz added a child revision: D6821: Add tests checking round-tripping of dir/rev/rel/snp objects generated by Hypothesis.Dec 9 2021, 5:28 PM

ardumont accepted this revision as: ardumont.Dec 10 2021, 10:30 AM

ardumont added 1 blocking reviewer(s): Reviewers.

LGTM (but see my comment)

swh/storage/postgresql/converters.py
110–112	maybe add a small comment on why the floor is needed. Also doesn't meth.floor return an int already?

This revision is now accepted and ready to land.Dec 13 2021, 10:21 AM

vlorentz added inline comments.Dec 13 2021, 10:43 AM

swh/storage/postgresql/converters.py
110–112	uh, you're right. I would have sworn it returned a float, but it looks like that was only on Python 2!

remove useless int(), add comment

Build is green

Patch application report for D6815 (id=24748)

Rebasing onto 7cb4128e40...

Current branch diff-target is up to date.

Changes applied before test

commit fb1b3a066ff0898318ea28c901761c5c3562f0f6
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Dec 9 16:25:20 2021 +0100

    postgresql: Fix one-by-one error in db_to_date on negative dates
    
    Using `int()` on `date.timestamp()` rounded it up (toward zero),
    but the semantics of `model.Timestamp` is that the actual time is
    `ts.seconds + ts.microseconds/1000000`, so all negative dates were
    shifted one second up.
    
    In particular, this causes dates from
    `1969-12-31T23:59:59.000001` to `1969-12-31T23:59:59.999999`
    (inclusive) to smash into dates from
    `1970-01-01T00:00:00.000001` to `1970-01-01T00:00:00.999999`,
    which is how I discovered the issue.

commit 34ca67e21f68de920f603223f629070683018c2a
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Dec 9 16:23:44 2021 +0100

    postgresql: Add tests for db_to_date.

See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/1504/ for more details.

Harbormaster completed remote builds in B25599: Diff 24748.Dec 13 2021, 1:02 PM

Closed by commit rDSTO34ca67e21f68: postgresql: Add tests for db_to_date. (authored by vlorentz). · Explain WhyDec 14 2021, 11:22 AM

This revision was automatically updated to reflect the committed changes.

vlorentz added a commit: rDSTO34ca67e21f68: postgresql: Add tests for db_to_date..