support crate 4.x

[chicco785]

fix #256

@c0c0n3 three test are failing.

• test_NTNE_limit probably because the query is actually wrong
• test_NTNE_offset may be actually a crate bug. i run the query manually and it seems that crate 4.x is not handling correctly offset when using it with an aggregate query (at least if in a union all select)
• test_bc well of course moving to crate 4.x it's not backward compatible with ql 0.5.1

[c0c0n3]

@chicco785 it looks like you've added these changes on top of PR #299. We should freeze this PR until we merge PR #299 into master, then rebase this PR onto master to avoid version control nightmares :-)

For the rest it looks like a very good start. What happens if you re-enable all reporter tests? Talking about this change:

- pytest src/reporter/ --cov-report= --cov-config=.coveragerc --cov-append --cov=src/
+ pytest src/reporter/tests/test_NTNE.py --cov-report= --cov-config=.coveragerc --cov-append --cov=src/

[chicco785]

yep, the idea was to rebase after merging the log one, locally i already re-enabled all report tests. i disable to focus on the troublesome ones

[chicco785]

I reported the Union all "bug" in crate github crate/crate#9854

[c0c0n3]

idea was to rebase after merging the log one, locally i already re-enabled all report tests

I see you've rebased the branch, good stuff, thanks!

disable to focus on the troublesome ones

cool bananas, just wanted to make sure the change was intentional :-)

reported the Union all "bug"

that's great, thanks!

[chicco785]

@c0c0n3
in general i assume that when you have lot of entities, offset is more important than order by, so at the time being for the crate bug, i removed order by.

i also improved the backward compatibility test, so we are able to test also database upgrade, and to test it across different combination of quantumleap and crate. see .travis.yml

[chicco785]

https://travis-ci.org/github/smartsdk/ngsi-timeseries-api/jobs/671718649?utm_medium=notification&utm_source=github_status highlights that query behaviour between crate versions in inconsistent

A theory could be that the order of result - in absence of order by - depends on the order of consolidation by crate (and not on the order of insertion).

[c0c0n3]

in general i assume that when you have lot of entities, offset is more important than order by, so at the time being for the crate bug, i removed order by.

That seems the most sensible thing to do at the moment. I opened #306 as a reminder to follow up on the Crate issue and possibly revert back to ordering the result set.

i also improved the backward compatibility test

That's fantabulous, thanks!

query behaviour between crate versions in inconsistent
A theory could be that the order of result - in absence of order by - depends on the order of consolidation by crate (and not on the order of insertion).

Yes probably. Since those tables don't have a PK, perhaps we get an implicit one with an associated index and results get output in index traversal order. But that's just a wild guess!

[c0c0n3]

Also, I'd update test_NTNE_limit to make it independent of result set ordering. What we want to test here is that the limit param is honoured. So the test should just check the returned tuple (id, type, ix) ∈ { (Room0, Room, room_ix), (Kitchen0, Kitchen, kitchen_ix) }.

[chicco785]

@c0c0n3 all test are passing

[chicco785]

@c0c0n3 this ensure that also when not timeindex is inserted, we use utc

[c0c0n3]

excellent thanks!

[chicco785]

@c0c0n3 unless there is anything else, this PR should be ready to land

[c0c0n3]

@chicco785 excellent, merged in!