Default query end time should be math.MaxInt64

[pauldix]

Too many people have reported bugs where they can't see their data because they wrote a data point with a future timestamp. This is because by default we set the query end time to time.Now() if they didn't specify an end time.

We should have the end time default to time.Unix(0, math.MaxInt64)

[otoolep]

We don't consider this a breaking change? Or we do, but we prepared to take the hit?

[pauldix]

Most people aren't writing data into the future. Also, it doesn't actually break the API from a client compatibility perspective. So I think it's worth doing. Also, the next release is 0.10.0

[otoolep]

This should be pretty straightforward one, and a chance to learn some of the parser and query code. It should be pretty clear in the code where the upper bound is set to now, if not explicit set in the query.

Most of the code changes will be in the tests.

[wind0r]

I think i am wrong but shouldn't changing this line be enough?
Even the test do not fail after changing it to max = time.Unix(0, math.MaxInt64).UnixNano()

[otoolep]

Yes, that should be all it takes. That's why it's low difficulty. :-)

Sounds like we need some more test coverage. What would be great is some tests added like this one:

https://github.com/influxdb/influxdb/blob/master/cmd/influxd/run/server_test.go#L913

basically a new test (TestServer_Query_NoTimeBounds) that writes a point very far into the future, another point far into the past, and then does a SELECT without specifying any time, and ensure all the data comes back. Today that test will fail, but after your fix, the test will pass. Make sense?

[wind0r]

OK =)
I will write a test and then create a PR.

[otoolep]

Great. Make sure your test fails first, before making any changes to the database code, so you can be sure your test is correct.

[beckettsean]

@pauldix this is a dupe of #4461. I'll let you decide which should be closed in favor of the other.

[pauldix]

@beckettsean ah right. Closed in favor of yours :)

[beckettsean]

@wind0r FYI we've closed this issue in favor of an older one. That one references both the upper bound and lower bound, so your very welcome PR is only a partial fix. Just mentioning that so we don't close #4461 if #5089 is merged.