Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues with temporal data, including (open) ranges / periods #155 #194

Merged
merged 7 commits into from
May 2, 2019

Conversation

cportele
Copy link
Member

resolves #155

@cportele cportele added this to the Part 1, Second Draft Release milestone Apr 11, 2019
@rcoup
Copy link

rcoup commented Apr 11, 2019

Conformance checks should probably include the correct behaviour around ranges (ie. everything is >=/<=). Presumably since fractions of seconds are optional in RFC3339 the ISO8601 range behaviour of truncating precision is desired?

eg. using the example range 1-2pm today UTC...

2019-04-11T12:00:00Z/2019-04-11T12:59:59Z equates to SQL (postgres for this example) of either

SELECT * FROM mytable
WHERE
  mytimestamp >= '2019-04-11T12:00:00Z'::timestamp
  AND date_trunc('second', mytimestamp) <= '2019-04-11T12:59:59Z'::timestamp;

or

SELECT * FROM mytable
WHERE
  mytimestamp >= '2019-04-11T12:00:00Z'::timestamp
  AND mytimestamp < ('2019-04-11T12:59:59Z'::timestamp + interval '1 second')
  -- ie. mytimestamp < 2019-04-11T13:00:00Z

(the latter will typically perform much better)

But following the ISO8601 range logic I think using the example range 2019-04-11T12:00:00.0Z/2019-04-11T12:59:59.9Z (ie. with some arbitrary second-fraction precision) should equate to SQL of:

SELECT * FROM mytable
WHERE
  mytimestamp >= '2019-04-11T12:00:00.0Z'::timestamp
  AND mytimestamp <= '2019-04-11T12:59:59.9Z'::timestamp;

Which deliberately misses anything from the last 100ms in the hour. So either the user needs the query timestamp precision to exceed the system/data timestamp precision (more 9s) or doesn't specify seconds fractions at all.

(yes, this is the annoying problem with inclusive ranges, but that ship appears to have sailed)

@m-mohr
Copy link
Contributor

m-mohr commented Apr 15, 2019

Why are inclusive ranges used? Wouldn't it be more convenient to do inclusive lower bound and exclusive upper bound?

@cportele
Copy link
Member Author

Why are inclusive ranges used? Wouldn't it be more convenient to do inclusive lower bound and exclusive upper bound?

The reason is that we are referencing ISO 8601 and the intervals include the limiting time instances. See #155 (comment)

@m-mohr
Copy link
Contributor

m-mohr commented Apr 15, 2019

Hmm, okay, seems we need to live with that.

For filtering we now have open date ranges, which is great! Nevertheless, it is still not possible in the responses, e.g. the temporal extents for the collections. So instead of updating

{
  "extent":{
    "temporal":{
      "interval": [["2015-06-23T00:00:00Z", "2015-06-24T00:00:00Z"]]
    }
  }
}

all the time, it would be great to allow:

{
  "extent":{
    "temporal":{
      "interval": [["2015-06-23T00:00:00Z", null]]
    }
  }
}

At least that was my main intention behind issue 155.

@cportele
Copy link
Member Author

@m-mohr - That makes sense to me. I will update the PR to allow that.

If anyone has concerns please let me know.

@cportele
Copy link
Member Author

@rcoup - I have updated the test description with the text below . It does not have the details of your post, but in general the abstract test suites do not really go into that detail. Maybe they should. For now, it is up to the executable test suite to specify and implement the tests in detail.

@cmheazel - please check my edits!

Use time intervals in the tests to verify the correct behavior for time intervals. In particular check that the start and end date-time are part of the interval (i.e. everything that is >= than the start and <= the end date-time is selected). Use also fractions of seconds in the tests.

@m-mohr - I have updated the #196 to support open intervals.

@cportele cportele merged commit 980a445 into master May 2, 2019
@cportele cportele deleted the issue-155 branch May 2, 2019 11:01
@rcoup
Copy link

rcoup commented May 2, 2019

Presumably since fractions of seconds are optional in RFC3339 the ISO8601 range behaviour of truncating precision is desired?

I think this is a key bit... if fractions of seconds are not provided, then the filter needs to include all values from that second to be compatible with ISO8601 ranges:

eg. '2019-04-11T12:34:56.7Z' means <= 56.700s
but '2019-04-11T12:34:56Z' means < 57s

@cportele
Copy link
Member Author

cportele commented May 2, 2019

Thanks @rcoup. I will add this to the text in another commit.

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.

Issues with temporal data, including (open) ranges / periods
3 participants