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

Implement ResultSet packing #13

Merged
merged 7 commits into from
Apr 13, 2021
Merged

Implement ResultSet packing #13

merged 7 commits into from
Apr 13, 2021

Conversation

desmondcheongzx
Copy link
Collaborator

No description provided.

@desmondcheongzx desmondcheongzx marked this pull request as draft April 3, 2021 17:08
@desmondcheongzx
Copy link
Collaborator Author

desmondcheongzx commented Apr 3, 2021

This pr addresses the error in #6. Additionally, it implements the predicate push-up optimisation as described in #5, i.e. we delay filtering data points as far as possible until the number of series to filter is minimised.

Query evaluation now works on the following test case. Say we're given 3 data points:

{"Write": {"name": "disk", "labels": {"hostname": "host_desmond"}, "variables": {"total": 10}, "timestamp": "2016-01-01 00:04:00+00:00"}}
{"Write": {"name": "disk", "labels": {"hostname": "host_desmond"}, "variables": {"total": 2}, "timestamp": "2016-01-01 00:05:00+00:00"}}
{"Write": {"name": "disk", "labels": {"hostname": "host_desmond"}, "variables": {"total": 4}, "timestamp": "2016-01-01 00:06:00+00:00"}}

And the following select query:

{"Select": {"name": "test", "predicate":{"name": "test","condition":{"Leaf":{"lhs": {"LabelKey": "hostname"},"rhs": {"LabelValue": "host_desmond"},"op": "Eq"}}}}}

This gives us the correct sorted results:

Received statement: Select { name: "test", predicate: Predicate { name: "test", condition: Leaf(Condition { lhs: LabelKey("hostname"), rhs: LabelValue("host_desmond"), op: Eq }) } }
Received result: [Record { name: "disk", labels: {"hostname": "host_desmond"}, variables: {"total": 1099511627776.0}, timestamp: 2016-01-01T00:04:00Z }, Record { name: "disk", labels: {"hostname": "host_desmond"}, variables: {"total": 2.0}, timestamp: 2016-01-01T00:05:00Z }, Record { name: "disk", labels: {"hostname": "host_desmond"}, variables: {"total": 4.0}, timestamp: 2016-01-01T00:06:00Z }]

. Additionally, we can evaluate more complex queries such as with an "AND" statement here

{"Select": {"name": "test", "predicate":{"name": "test","condition":{"And":[{"Leaf":{"lhs": {"LabelKey": "hostname"},"rhs": {"LabelValue": "host_desmond"},"op": "Eq"}},{"Leaf":{"lhs": {"Variable": "total"},"rhs": {"Metric": 4},"op": "Lt"}}]}}}}

which returns the correct single data point:

Received statement: Select { name: "test", predicate: Predicate { name: "test", condition: And(Leaf(Condition { lhs: LabelKey("hostname"), rhs: LabelValue("host_desmond"), op: Eq }), Leaf(Condition { lhs: Variable("total"), rhs: Metric(4.0), op: Lt })) } }
Received result: [Record { name: "disk", labels: {"hostname": "host_desmond"}, variables: {"total": 2.0}, timestamp: 2016-01-01T00:05:00Z }]

@desmondcheongzx desmondcheongzx linked an issue Apr 3, 2021 that may be closed by this pull request
@desmondcheongzx desmondcheongzx marked this pull request as ready for review April 3, 2021 20:14
@desmondcheongzx
Copy link
Collaborator Author

@n-young rebased and ready for review!

Comment on lines +138 to +140
r1.union(r2, shared_block);
r1.unpack(shared_block);
r1
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Union returns an unpacked, so this unpack shouldn't do anything right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah not necessarily: if a union encounters two packed ResultSets, AND both ResultSets don't have filters, then it doesn't unpack them.

@n-young
Copy link
Owner

n-young commented Apr 13, 2021

LGTM! Sorry this took so long to review :(

@n-young n-young merged commit b418d8d into n-young:master Apr 13, 2021
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.

Error in query evaluation
2 participants