Opened at 2015-09-14T16:28:00Z
Closed at 2016-04-04T14:25:12Z
#2505 closed defect (fixed)
Magic Folder: consider types of (size, ctime, mtime) more carefully
Reported by: | daira | Owned by: | daira |
---|---|---|---|
Priority: | normal | Milestone: | undecided |
Component: | code-frontend-magic-folder | Version: | 1.10.1 |
Keywords: | blocks-magic-folder-merge | Cc: | |
Launchpad Bug: |
Description
These need to be compared, so it's important that the magic folder db be able to represent them precisely enough.
Change History (11)
comment:1 Changed at 2015-09-25T01:27:55Z by daira
comment:2 Changed at 2015-12-03T19:29:21Z by daira
- Keywords blocks-magic-folder-merge added; otf-magic-folder-objective4 removed
- Status changed from new to assigned
comment:3 Changed at 2016-01-18T20:39:25Z by daira
The issue here is, for all possible floats f that can be returned by time.time(), does storing f in a database field and then retrieving it result in a float == to f? Since we compare timestamps using ==, if this were not the case, then we'd detect spurious timestamp mismatches.
comment:4 Changed at 2016-01-18T20:42:56Z by daira
I don't know where the type NUMBER came from in the current schema. We probably want REAL.
comment:5 follow-up: ↓ 7 Changed at 2016-01-19T14:42:43Z by dawuud
- Status changed from assigned to new
ok here's my solution: https://github.com/david415/tahoe-lafs/tree/2505.db-schema-types.0
i've replaced NUMBER and TIMESTAMP with REAL. According to this link, TIMESTAMP is useful if you sometimes want to have a default value of the current time; we do not.
https://androidcookbook.com/Recipe.seam?recipeId=413
All tests pass. Review needed.
comment:6 Changed at 2016-01-19T17:51:20Z by meejah
dawuud's changes look good to me; certainly an improvement over what's there now.
I do share the == concern about floats -- *especially* because Python's internal representation isn't necessarily an 8-byte IEEE as SQLite's is.
Also, although time.time() does return a float, we really only care about seconds so wouldn't lose anything by storing an INTEGER in the database and always doing int(time.time()) when comparing timestamps (that is, throwing away fractional seconds). Also, within Twisted, it's far better to use IReactorTime.seconds() anyway. So whenever it's convenient (e.g. whenever we have a reactor instance to call .seconds() on) we should probably prefer that. As a side-effect, this makes testing things that use that interface properly way easier.
Personally, I think the integer based approach is far less likely to have weird errors or rounding issues etc. and is thus overall way safer.
comment:7 in reply to: ↑ 5 Changed at 2016-01-21T15:36:57Z by daira
Replying to dawuud:
ok here's my solution: https://github.com/david415/tahoe-lafs/tree/2505.db-schema-types.0
i've replaced NUMBER and TIMESTAMP with REAL. According to this link, TIMESTAMP is useful if you sometimes want to have a default value of the current time; we do not.
https://androidcookbook.com/Recipe.seam?recipeId=413
All tests pass. Review needed.
Reviewed.
comment:8 Changed at 2016-01-21T17:12:55Z by dawuud
pushed changes here: https://github.com/david415/tahoe-lafs/tree/2438.magic-folder-stable.9
comment:9 Changed at 2016-01-21T17:13:09Z by dawuud
- Resolution set to fixed
- Status changed from new to closed
comment:10 Changed at 2016-03-29T14:53:48Z by daira
- Resolution fixed deleted
- Status changed from closed to reopened
comment:11 Changed at 2016-04-04T14:25:12Z by daira
- Resolution set to fixed
- Status changed from reopened to closed
We changed the types to integer nanoseconds (currently this is only on the 1431.windows-fixes.3 branch).
https://www.sqlite.org/datatype3.html