Skip to content

Loading…

Provide optional "sort" and "limit" parameters to query #125

Closed
eric opened this Issue · 15 comments

5 participants

@eric

It would be nice, when querying the index, to be able to specify a sort order for events and a limit on the number of events that were returned.

@algernon

I attempted to do this, in two ways for now: first I tried to abuse the grammar and meta-data, but I utterly failed. And then I realized that this whole thing is using protocol buffers! So the next option is to extend the Query proto to have optional sort and limit members. But for that, I need to reach into riemann-java-client, because riemann is using that to get the protobuf stuff from.

I'll likely get around to do that this week. I already have the necessary changes in index.clj, and the transport.clj part should be fairly easy too.

I'm not exactly sure how sort would work, though, so I'll do limit first, because that sounds reasonably simple, and that's what I would like to use in the first place.

@aphyr
@algernon

Hrm... that means I have to learn antlr. Too bad. I'll see what I can do, though. When placing it in the grammar, what syntax would be best?

I was thinking something along these lines:

limited-to 5 and service = "something"

Or, well, the limited-to could be anywhere. The hard part, though, is getting the information out of the ast. Without changing how it all works too much, I'd have two ideas: either set some meta-data on what query/ast returns, so that transport/handle can look at it and act accordingly, or have a pass through the ast to find limited-to, remember the setting and pull it out.

@aphyr
@algernon

Clojure expressions would make this a whole lot easier. But that'd also open up a can of worms, wouldn't it? I kinda like the current simple grammar, I just need to learn a bit of antlr-esque, I think.

As for clj-antlr: I can wait. If I desperately need limit-like functionality, I'll just patch the code for the time being, but my dataset isn't big enough to need that yet (it will grow, but that takes a couple of months - I just like to plan ahead).

So, I'll revisit this feature sometime in the next few days, and see where I can take it, if that's ok.

@aphyr
@algernon

I have a solution that works okay-ish, so far. Syntax is limit 5 (host = "some-host" and service =~ "foo%"). The reason limit comes first is so that I can turn that into an AST with limit as head, which I can then pull apart later, and give the limit to a take.

Still need to figure out how to only allow a top-level limit, but not subsequent ones, because that makes no sense.

@algernon algernon added a commit to algernon/riemann that referenced this issue
@algernon algernon Implement simplistic query limiting
This adds a new "limit N ..." construct to the query grammar. When a
query starts with "limit N", it will be limited to N results, in
whatever order they're found. Otherwise all results are returned.

This addresses part of #125.

Signed-off-by: Gergely Nagy <algernon@madhouse-project.org>
258bbab
@aphyr

I think you're on the right track here @algernon, but we should allow limit to occur after the query; I guarantee folks are going to put it there, haha. :)

@aphyr

I guess we also need to figure out the semantics for limiting a streaming query; my intuition is that should be an error, unless we want to reconstruct a top-k ordering state machine in the streaming query itself.

@algernon

Yeah, allowing limit at the end would be nice, but my brain hurts from all the antlr stuff. I'll revisit that a bit later. :)

@pyr
pyr commented

@algernon should we keep this open or can we table it for now?

@algernon

Feel free to close this, I don't see myself getting around to update this anytime soon. When I do, I have my branch.

@eric

It's definitely something that I run into wanting quite a bit, but definitely agree there's a lot of things that need to happen to make it make sense holistically.

@pyr
pyr commented

@eric agreed.

@pyr pyr closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.