Implement new Kibana query language #12624

Bargs · 2017-06-30T21:40:51Z

Replaces #11915 (changes from that PR are in the first commit)
Fixes #12282
Part of #10789

See the issue description for an up to date reference for the new syntax. This PR has everything in it that I think is necessary for 6.0 beta1.

To enable query language switching, go to Advanced Settings and enable search:queryLanguage:switcher:enable. You can also set kuery as the default language by updating search:queryLanguage.

Some caveats:

Doesn’t support filters created via filters agg. This would be a nice to have, but since we only just started supporting filters from the filter agg at all, I don't see it as a blocker. Need some time to think about how we should do it. I don't really want to add support for query_string_query and raw query DSL natively in kuery. Including them as aliased sub-queries might be one option.
There's no way to migrate saved searches written in lucene.
Geo queries can get quite long very quickly. It's pretty obvious we'll need multi-line support. I'd like to explore this in another PR in the near future.

TODO:

Add a crapload of tests

Release Note: This PR adds a new experimental query language to Kibana. The new language is turned off by default but can be enabled in the Management > Advanced Settings via the search:queryLanguage:switcher:enable option. Changing this setting to true will add a language selector to the query bar that allows users to choose the new language for their query. Details of the new query language can be found in the issue description (#12282).

Bargs · 2017-06-30T21:48:12Z

src/core_plugins/kibana/public/dashboard/dashboard.js

I moved all the filter handling logic into the query_manager module, but there's still a bit of tech debt here with this watch. I've got another branch where I'm working on removing the need for a watch, but I don't think it's necessary for this PR.

weltenwort · 2017-07-03T13:48:12Z

After playing around with it a bit I have to say it already feels more solid that the lucene query syntax 😉 Here are a few observations though:

Maybe being more lenient with whitespace would be good: ( range:[1 to 10] ) is allowed but (range:[ 1 to 10 ]) or range:[1 to 10] is not
At first I was confused that (is(extension, jpg)) (and) (is(response, 200)) is valid until I realized it is interpreted as is(extension, jpg) and is(<ANYFIELD>, and) and is(response, 200). Maybe we should provide some kind of feedback to the user to make that clear. I was thinking of maybe automatically reformatting the query to a canonical, unambiguous form. But maybe that is too invasive?
It was surprising to notice that *:"win xp" and machine.os:"win xp" are interpreted differently (the former is a query_string query with all its syntax and the latter a match_phrase query). This allows to escape kuery using quotes: "_exists_:host". Is that on purpose?
To me the range query shorthand a:[b TO c] feels like it should be inclusive, i.e. equivalent to range(a, gte=b, lte=c).

Overall it feels quite intuitive so far. I will dig into the code next.

Bargs · 2017-07-03T16:11:00Z

Good points @weltenwort, you've touched on some important and thorny issues.

At first I was confused that (is(extension, jpg)) (and) (is(response, 200)) is valid until I realized it is interpreted as is(extension, jpg) and is(, and) and is(response, 200). Maybe we should provide some kind of feedback to the user to make that clear. I was thinking of maybe automatically reformatting the query to a canonical, unambiguous form. But maybe that is too invasive?

This is a tricky subject for sure. I'm a bit wary of automatic reformatting. As a user I'm always a bit surprised and confused when an app does this to me. An option to manually kick off an auto-reformat would be cool, but maybe that's a feature for another PR.

I think the simplest option for now would be to consider and and or keywords of the query language. The example you provided would then throw a parse error. If the user actually wanted to query for the word "and" or "or", they would need to wrap it in double quotes. How does that sound?

The nuclear option would be to require double quotes around all search terms, as you mentioned in the last PR. I'm not quite ready to pull that trigger yet since no-one likes typing double quotes constantly.

It was surprising to notice that *:"win xp" and machine.os:"win xp" are interpreted differently (the former is a query_string query with all its syntax and the latter a match_phrase query). This allows to escape kuery using quotes: "exists:host". Is that on purpose?

Not on purpose :/ I obviously didn't think this one all the way through. I'm just abusing the query_string_query for its all_fields mode. I initially tried using the multi_match query with "fields": ["*"] but got the error The _source field is not searchable, so I switched to the query_string_query. Any idea how to execute a match query across all fields? My research is coming up nil, so I may need to reach out to the ES team on this one. Perhaps @dakrone could impart some wisdom.

To me the range query shorthand a:[b TO c] feels like it should be inclusive, i.e. equivalent to range(a, gte=b, lte=c)

Me too, this is a bug :) good catch!

Bargs · 2017-07-03T17:47:29Z

@weltenwort I've addressed everything except for the query_sting_query issue

weltenwort · 2017-07-05T16:22:17Z

Regarding the phrase search across all fields: I imagine we could use a query_string_query and wrap it in quotes (\-escaping user-supplied quotes) to force a phrase search. I cannot judge whether there are other caveats with that approach though.

dakrone · 2017-07-05T16:25:54Z

I initially tried using the multi_match query with "fields": ["*"] but got the error The _source field is not searchable, so I switched to the query_string_query. Any idea how to execute a match query across all fields? My research is coming up nil, so I may need to reach out to the ES team on this one. Perhaps @dakrone could impart some wisdom.

I am familiar with this, I tried this initially when working on the all_fields mode with the same problem, the issue is that a blanket wildcard includes some fields that are not (and should not) be searchable.

Adrien opened an issue to allow fields:* queries elastic/elasticsearch#25551, which internally can rewrite to the correct combination of fields (we do this now with all_fields by having a list of "valid" fields and field-types that should be included in the field list)

Bargs · 2017-07-05T16:51:00Z

@dakrone thanks for the info, you guys are great!

@weltenwort I'll try your suggestion as a temporary measure, I think it should work alright until we can do a proper match query across all fields.

Bargs · 2017-07-06T21:41:55Z

Note: function syntax (e.g. is(response, 200)) is slightly broken at the moment, I'm in the middle of extracting the AST node creation code out of the grammar file and writing grammar tests. I'll get this finished up tomorrow.

weltenwort

I though I would already submit this first half of the code review in the interest of not losing any unsubmitted comments. 😉 More to come later today.

weltenwort · 2017-07-07T13:12:11Z

src/ui/public/filter_bar/lib/map_geo_polygon.js

+  return function (filter) {
+    if (filter.geo_polygon) {
+      return courier
+      .indexPatterns


The style guide prescribes indentation of chained method calls for readability:
(https://github.com/elastic/kibana/blob/master/style_guides/js_style_guide.md#chaining-operations)

I have to object here. I'm following the existing style of all the other files in this module. Changing just this one file makes it inconsistent and harder to read in context. We either need to revert this indentation rule (which was thoughtlessly changed) or fix the entire codebase in one go, otherwise it's just making things worse. I'll open a separate issue to discuss this.

weltenwort · 2017-07-07T13:15:34Z

src/core_plugins/kibana/public/dashboard/dashboard.js

-        }
+        // These are handled by the filter bar directive when lucene is the query language
+        Promise.all(filters.map(queryManager.addLegacyFilter))
+        .then(updateState)


The style guide prescribes indentation of chained method calls for readability:
(https://github.com/elastic/kibana/blob/master/style_guides/js_style_guide.md#chaining-operations)

I've updated this spot to maintain consistency with the rest of this particular file

30029eb

weltenwort · 2017-07-10T10:36:52Z

src/ui/public/kuery/lib/filter_migration/index.js

-
-  return negate ? nodeTypes.function.buildNode('not', node) : node;
-}
+export { filterToKueryAST } from './lib/filter_to_kuery';


The path public/kuery/lib/filter_migrations/lib/filter_to_kuery means it is a lib inside a lib inside the public lib folder. public already is a library folder itself, so the libraries inside could have sibling relations instead of nesting them further. Attempting to reduce the nesting depth seems advisable to me.

I feel like shoving everything into the top level public directory makes it very difficult to understand the most important relationships between modules. For example, ask a newcomer to infer the relationships of all the vislib code scattered throughout public and they're gonna have a bad time.

Let me propose a couple options that cut down on the /lib/lib/libness while still grouping all the kuery code in one spot.

Option 1

Option 2

👍 for the second option

I agree that putting everything on the top level would make code paths too hard to trace.

👍 0a83211

weltenwort · 2017-07-10T10:42:01Z

src/ui/public/kuery/lib/filter_migration/lib/__tests__/exists.js

+      expect(result.arguments[0].value).to.be('foo');
+    });
+
+    it('should return undefined if the given filter is not of type "exists"', function () {


From the api design point of view I would expect the function to throw an exception in that case. I have seen that other code uses the undefined to conditionally chain calls, but it might be worth expressing that differently to keep the apis clean.

Returning undefined and throwing an exception for a non-exceptional scenario both seem equally hacky to me. Since these are private functions used only by filterToKueryAST, it doesn't seem terribly important, does it?

If you're strongly against returning undefined, perhaps I should add a supports function to each converter module that returns true/false depending on whether that module supports the given filter?

If we had a type system passing something different from an "exists" filter as an argument would be a type error. So I would absolutely call this an exceptional scenario. Returning undefined just has no intrinsic meaning and can be caused by all kind of accidental mistakes in JS. I would be in favor of clearly signalling "here is your converted filter" and "I can not convert that". Adding a supports function vs throwing an exception essentially comes down to the "ask permission rather than forgiveness" pattern. I would be fine with either.

But we don't have a type system :) an exceptional scenario in one language is another language's bread and butter. Anyway, it doesn't matter much to me since this isn't a public API. I'll just switch them to exceptions since it'll be easier than adding a new function.

thanks 😄 just want to point out that even though there is no way in javascript to write down function signatures or even enforce them, the functions still have one.

Personally, as a developer who has to reconstruct those signatures in his head on the fly, I always find undefineds a major pain. Every piece of code that works with the results has to check for it and whenever I return to the code I have to derive the meaning of undefined again. I appreciate this freeing up of cognitive capacity. 🦄

I suppose it's just one of those things that comes down to personal preference. I have the exact same feelings about exceptions XD

If only javascript had proper interfaces and polymorphism it would be a non-issue because we wouldn't have to write this hacky switching logic ourselves.

weltenwort · 2017-07-10T10:53:18Z

src/ui/public/kuery/lib/filter_migration/lib/__tests__/filter_to_kuery.js

+      const result = filterToKueryAST(filter);
+
+      expect(result).to.have.property('type', 'function');
+      expect(result).to.have.property('function', 'not');


Should we maybe also test that the argument is passed to the not function?

👍 aa2c478

weltenwort · 2017-07-10T11:45:29Z

src/ui/public/kuery/lib/functions/is.js

+}
+
+export function toKueryExpression(node) {
+  if (node.serializeStyle !== 'operator') {


Shouldn't the type be shorthand here? (With foo:bar being shorthand, is(foo, bar) being function and foo is bar being operator.)

foo is bar isn't valid. I think of : as a binary operator. I think my use of the term shorthand for the range alias :[] may be confusing things here. I didn't use the term operator there because it's a weird syntax that sort of half wraps the arguments, I don't think most people would think of it as an operator. But we could call it an operator and get rid of the shorthand term entirely, if you think that would be less confusing.

I know foo is bar is not valid in the current implementation. But one might expect it to be. In any case, I just tried to delineate the different cases by giving examples.

weltenwort · 2017-07-10T11:46:00Z

src/ui/public/kuery/lib/functions/not.js

+}
+
+export function toKueryExpression(node) {
+  if (node.serializeStyle !== 'operator') {


This looks like a shorthand to me.

It's a unary operator in prefix notation.

You could make the same argument for :. (being a binary operator in infix notation instead of a "shorthand")

Yes, that's exactly the argument I made about : above :) I wonder if you're looking at an outdated version of the PR? I think at one point I categorized : as shorthand, but I've since updated it to operator.

I'll try to sum up my thoughts here since I can't do an inline reply to your review summary below. I don't believe I've used these terms inconsistently. I think we're just operating with different definitions in mind. Here's how I categorize things:

implicit (just and)

operator (foo:bar, foo or bar, foo and bar, -foo) (basically anything the grammar has a special case for)

function (is(foo, bar))

shorthand (just range bytes:[1000 to 8000])

I wouldn't be surprised if these categorizations evolved along with the language, but they seem to work ok for now.

Yes, just having the definitions written down does a lot for the understanding. 👍

I still don't get why : and :[ to ] are different categories, but maybe future additions will make that distinction clearer to me.

:[ to ] just seemed weird to me... it felt wrong to call it an operator since it's not strictly prefix or infix. However, after giving it some more thought and perusing the Operator Wikipedia page, maybe it's ok. It's not much weirder than the ternary operator. I thought calling it shorthand instead of operator would reduce confusion, but it seems that's not the case ;)

weltenwort · 2017-07-10T11:54:20Z

src/ui/public/kuery/lib/__tests__/ast.js

+      expect(ast.toKueryExpression()).to.be('');
+
+      const node = nodeTypes.function.buildNode('exists', 'foo');
+      delete node.type;


What about a type that is given but does not match any known ast node type?

👍 7b1c4e9

weltenwort · 2017-07-10T11:55:00Z

src/ui/public/kuery/lib/__tests__/ast.js


-      expect(parseResult).to.eql(ast);
+      const node = nodeTypes.function.buildNode('exists', 'foo');
+      delete node.type;


What about a type that is given but does not match any known ast node type?

👍 7b1c4e9

weltenwort · 2017-07-10T11:57:31Z

src/ui/public/kuery/lib/__tests__/ast.js

+  describe('symmetry of to/fromKueryExpression', function () {

-      expect(parseResult).to.eql(ast);
+    it('converting from an expression to an AST and back again should result in the same expression', function () {


one could say "toKueryExpression and fromKueryExpression should be inverse operations"

👍 293279e

weltenwort

So from what I saw is that we have 4 serialization styles:

implicit (only the implicit and)
operator (e.g. foo or bar, other languages call this "infix notation")
function (e.g. is(foo, bar), otherwise called "prefix notation")
shorthand (e.g. foo:bar)

As I commented inline, there seem to be some inconsistencies in assigning these labels to the styles. In addition, I think we should choose a consistent style to use by default and in toKueryExpression.

Another inconsistency that slightly bugs me is the varying use of literal and named arguments in the AST. Is there something keeping us from always using named arguments? I expect that would make maintaining backwards-compatibility easier when we expand the language if the order-dependent logic is restricted to only the topmost parsing layer.

In the individual toKueryExpression functions there seems to be a lot of duplication. And asymmetric looking code. Maybe some kind of mapping from serialization style to a formatting function could make that more readable. But that's obviously a nice-to-have that can be delayed.

weltenwort · 2017-07-10T13:39:22Z

src/ui/public/kuery/lib/functions/__tests__/and.js

+
+    describe('toKueryExpression', function () {
+
+      it('should serialize "and" nodes with an implicit syntax by default', function () {


Explicit is better than implicit

is one of my favourite lines from the zen of python. I think we should promote being explicit and unambiguous whenever possible.

👍 a6f80f1

weltenwort · 2017-07-10T13:51:17Z

src/ui/public/kuery/lib/functions/__tests__/is.js

+        expect(result.simple_query_string.query).to.be('"+response"');
+      });
+
+      it('should return an ES exists query when no value is provided', function () {


That means there are two ways of creating an exists clause: The explicit exists(foo) and the implicit is(foo). The latter opens up opportunities for misunderstanding and wrong expectations (e.g. to me this reads as if foo is evaluated as a boolean). Maybe we should require the value argument for the is type?

Agreed, both arguments should be required. I think this was left over from some intermediate implementation.

b73bd2b

weltenwort · 2017-07-10T14:09:39Z

src/ui/public/kuery/lib/functions/__tests__/range.js

+        const result = range.buildNodeParams('bytes', givenParams);
+        const { arguments: [ , ...params ] } = result;
+
+        params.map((param) => {


If params was empty no expectation would be executed.

👍 07804f2

weltenwort · 2017-07-10T14:47:42Z

src/ui/public/kuery/lib/functions/and.js

+    bool: {
+      filter: children.map((child) => {
+        if (child.type === 'literal') {
+          child = nodeTypes.function.buildNode('is', '*', child.value);


I wonder if this is actually the right place to build that node. Shouldn't that have been created in buildNodeParams?

I don't think so, that would alter the AST built from query string provided by the user. Then when we call toKueryExpression on this node we won't know if the original query string contained an explicit is function, or just a literal.

In my experience so far, it seems best to read the query string as-is with minimal "interpretations" at parse time and leave things like interpreting the meaning of a literal argument to the individual serializers.

Hm, or we could introduce an "implicit" style for is, which would preserve that formatting while still having an unambiguous AST. I don't have enough experience with building languages to judge the pros and cons of either solution. My intuition is that we should either create a fully "interpreted" AST as early as possible or as late as possible, but not in between.

I don't have enough experience with building languages to judge the pros and cons of either solution.

Nor do I ¯\(ツ)/¯

Usually when faced with such uncertainty I'll just move ahead with whatever works and assume time will reveal a better solution. Let's leave as-is for now if you're ok with it.

agreed, just something to keep in mind

weltenwort · 2017-07-10T14:52:18Z

src/ui/public/kuery/lib/functions/geo_bounding_box.js

+export function buildNodeParams(fieldName, params) {
+  params = _.pick(params, 'topLeft', 'bottomRight');
+  const fieldNameArg = nodeTypes.literal.buildNode(fieldName);
+  const args = _.pairs(params).map((argument) => {


The callback given to _.map() on an object actually receives the value and the key, so pairs should not be required.

👍 a54274a

weltenwort · 2017-07-10T15:03:40Z

src/ui/public/kuery/lib/functions/is.js

+}
+
+function isDoubleQuoted(str) {
+  return (_.first(str) === '"') && (_.last(str) === '"');


This works, but using str.startsWith('"') might be more idiomatic.

👍 69447bf

weltenwort · 2017-07-10T15:05:00Z

src/ui/public/kuery/lib/functions/is.js

+      }
+    };
+  }
+  else if (fieldName !== '*' && _.isUndefined(value)) {


As commented earlier, I am not sure this is a good idea.

👍 b73bd2b

weltenwort · 2017-07-10T15:07:31Z

src/ui/public/kuery/lib/functions/not.js

+export function toElasticsearchQuery(node, indexPattern) {
+  let [ argument ] = node.arguments;
+  if (argument.type === 'literal') {
+    argument = nodeTypes.function.buildNode('is', '*', argument.value);


Similar to the other case, I feel this should have happened earlier (probably in buildNodeParams).

See comment above

weltenwort · 2017-07-10T15:10:45Z

src/ui/public/kuery/lib/functions/not.js

+  const [ argument ] = node.arguments;
+  const queryString = ast.toKueryExpression(argument);
+
+  if (argument.function && (argument.function === 'and' || argument.function === 'or')) {


It might be less coupled to and and or specifically if this looked at the serialization style to decide whether it is an infix that needs grouping or whether it is a "self-grouping" prefix style expression. That way it would also leave out the parentheses with prefix-style ands and ors.

I think you're right, this will need to become smarter eventually. I don't have the granularity required in serializeStyle for what you describe yet, and I'm not quite ready to dive into overhauling that. I think the best solution will become more clear to me over time, so I'd like to defer this until later.

Forgot to mention: I updated this condition to check for the function serializeStyle, so that in the meantime we won't run into the unnecessary wrapping you described.

03ed922

weltenwort · 2017-07-10T15:13:54Z

src/ui/public/kuery/lib/functions/range.js

+
+export function toElasticsearchQuery(node, indexPattern) {
+  const [ fieldNameArg, ...args ] = node.arguments;
+  const fieldName = ast.toElasticsearchQuery(fieldNameArg);


In other places the fieldName is hardcoded to be a literal. Is this case different on purpose?

No real reason, it should probably be literal everywhere
ec1ae47

weltenwort · 2017-07-10T17:07:54Z

src/ui/public/query_manager/index.js

+
+  return function (state) {
+
+    function add(field, values = [], operation, index) {


And put your optional arguments at the end.

-- Kibana JavaScript Style Guide

Aside from that, maybe we should take this chance to improve this function's signature? Or is it meant to be a compatibility adapter?

The idea is that this will become the API we pass to clients who want to manipulate the query/filters. In particular, the vis team needs an API to pass to visualizations. Mostly I was being lazy here because I'm going to have to revisit and expand this API before handing it to the vis team. I figured I'd get a better idea of what the function signatures should look like once I start working on the rest of the API.

lukasolson

Not sure if this is just me, but seems like we're missing the outline on the drop-down here:

lukasolson

When your search doesn't return results, if you have Kuery selected, the message still says the message about the query bar using Lucene syntax with examples. Not exactly sure if we want to just hide it or show how to use Kuery there.

lukasolson

When I create a visualization and use the filters bucket aggregation, I get errors. If I have an empty input, I get this error:

TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at _abstract.js:394
    at Array.forEach (<anonymous>)
    at recurse (_abstract.js:387)
    at _abstract.js:401
    at processQueue (angular.js:14745)
    at angular.js:14761
    at Scope.$eval (angular.js:15989)
    at Scope.$digest (angular.js:15800)
    at angular.js:16028

And when I type something into the filters input, I get this:

Error: [parsing_exception] [_na] query malformed, must start with start_object, with { line=1 & col=89 }
    at respond (https://localhost:5601/pni/bundles/kibana.bundle.js?v=8467:16995:17)
    at checkRespForFailure (https://localhost:5601/pni/bundles/kibana.bundle.js?v=8467:16954:9)
    at https://localhost:5601/pni/bundles/kibana.bundle.js?v=8467:1823:9
    at processQueue (https://localhost:5601/pni/bundles/commons.bundle.js?v=8467:37815:29)
    at https://localhost:5601/pni/bundles/commons.bundle.js?v=8467:37831:28
    at Scope.$eval (https://localhost:5601/pni/bundles/commons.bundle.js?v=8467:39059:29)
    at Scope.$digest (https://localhost:5601/pni/bundles/commons.bundle.js?v=8467:38870:32)
    at Scope.$apply (https://localhost:5601/pni/bundles/commons.bundle.js?v=8467:39167:25)
    at done (https://localhost:5601/pni/bundles/commons.bundle.js?v=8467:33616:48)
    at completeRequest (https://localhost:5601/pni/bundles/commons.bundle.js?v=8467:33814:8)

Bargs · 2017-07-11T20:51:58Z

@weltenwort I believe I've updated or responded to all of your inline feedback. A couple of things you mentioned in your summary:

Another inconsistency that slightly bugs me is the varying use of literal and named arguments in the AST. Is there something keeping us from always using named arguments? I expect that would make maintaining backwards-compatibility easier when we expand the language if the order-dependent logic is restricted to only the topmost parsing layer.

Are you saying we shouldn't support unnamed parameters in the language, or that unnamed parameters in a query string should still turn into a namedArg node in the AST?

If the former: Less verbosity and more fluid typing are the main arguments for unnamed parameters, I think. is(foo, bar) is a lot easier to type than is(fieldName=foo, value=bar)

If the latter: It's a possibility I've considered. I suppose we could give namedArg nodes a serializeStyle like functions have. It just doesn't seem terribly necessary at the moment. I figured I'd play around with the idea in a future refactoring.

In the individual toKueryExpression functions there seems to be a lot of duplication. And asymmetric looking code. Maybe some kind of mapping from serialization style to a formatting function could make that more readable. But that's obviously a nice-to-have that can be delayed.

Agreed, it would be nice to DRY up these functions in a future refactoring after the syntax has seen some use and we're sure this is the way we want to go.

Thanks for all the feedback so far, keep it coming!

weltenwort · 2017-07-12T09:53:30Z

Are you saying we shouldn't support unnamed parameters in the language, or that unnamed parameters in a query string should still turn into a namedArg node in the AST?

I mean the latter. That way the interpretation of the position can happen early and only once. The idea of having a single "argument" AST node type with a style property sounds good to me, probably worth trying out in a subsequent PR.

Bargs · 2017-07-12T15:22:19Z

@lukasolson thanks for the feedback, I've made some updates:

When your search doesn't return results, if you have Kuery selected, the message still says the message about the query bar using Lucene syntax with examples.

Good catch. For now I've simply updated the no-results page to hide the bit about the lucene query syntax if kuery is selected in the query bar. Eventually I do think we'll want to fill this out with some details about kuery.

When I create a visualization and use the filters bucket aggregation, I get errors.

This was due to my changes to from_user.js. I didn't realize the filter agg was using our parse-query directive. I've updated the filter agg's serializer to correctly wrap lucene query strings in json just like we do when building a query from the query bar.

Not sure if this is just me, but seems like we're missing the outline on the drop-down here

This seems to be part of the UI framework (see screenshots from the framework docs below). If it's a mistake (cc @cjcenizal) we should probably fix it in a separate PR.

I believe this addresses all of your points. Please take another look!

lukasolson

Pretty sure this is related to the match_all changes, but if you create a filters aggregation, the label in the visualization will be {"match_all":{}}

Bargs · 2017-07-20T15:17:25Z

@lukasolson I fixed the issues with the range filter and the filters agg label. or and and are both case insensitive. Can you share the query you're having trouble with? Perhaps there's something else odd going on.

Bargs · 2017-07-20T16:28:36Z

Test failure is unrelated to this PR

lukasolson · 2017-07-20T17:03:42Z

Can you share the query you're having trouble with? Perhaps there's something else odd going on.

It's a simple OR with conditions on either side:

Bargs · 2017-07-20T17:30:21Z

@lukasolson ah yep, I see what's wrong, looks like I was deceiving myself into thinking it was working 😅

Should be fixed now with my latest commit

lukasolson

I'm seeing a bit of a funky behavior. Here's the steps to reproduce:

Select lucene as the query language
Create a bar chart and bucket by terms
Switch to kuery
Click on one of the bars to filter by the term
Click back, then back again

When you click back the second time, you'll be back to lucene, but the filter will be applied.

Bargs · 2017-07-20T19:21:59Z

@lukasolson I've run into that before, it's because of how the silly $newFilters scope variable is used. When I tried to fix it in the past I just ended up creating an infinite digest loop. I just gave it another shot and I think I got it this time.

lukasolson

LGTM! 💯 🎆 🥇 🎈

Bargs · 2017-07-21T14:53:32Z

The test that failed passed locally.... probably a timing issue

jenkins, test this

ppisljar · 2017-07-24T06:54:14Z

🎉

jimgoodwin · 2017-07-25T19:24:58Z

@Bargs Please update the description above to have at least one high quality screen shot and a Release Note: paragraph at the end with a user readable summary.

Bargs added :Discovery review v6.0.0-beta1 labels Jun 30, 2017

Bargs requested review from lukasolson, thomasneirynck and weltenwort June 30, 2017 21:40

This was referenced Jun 30, 2017

Support multiple query languages in the query bar #11915

Closed

Build a Better Search Experience #10789

Closed

Bargs commented Jun 30, 2017

View reviewed changes

Bargs force-pushed the enhanceKuery branch from c44a844 to 54420f3 Compare July 3, 2017 17:45

Bargs mentioned this pull request Jul 5, 2017

Remove all_fields:true in favour of fields:* elastic/elasticsearch#25551

Closed

dakrone mentioned this pull request Jul 5, 2017

Default query to match_all instead of query_string #12583

Closed

weltenwort reviewed Jul 10, 2017

View reviewed changes

Bargs mentioned this pull request Jul 10, 2017

[Styleguide] Revert chain indentation rule or update the entire codebase #12729

Closed

weltenwort reviewed Jul 10, 2017

View reviewed changes

lukasolson requested changes Jul 11, 2017

View reviewed changes

thomasneirynck removed their request for review July 11, 2017 19:49

lukasolson requested changes Jul 19, 2017

View reviewed changes

lukasolson reviewed Jul 20, 2017

View reviewed changes

Bargs force-pushed the enhanceKuery branch from aa24d92 to d71d0c6 Compare July 20, 2017 20:44

lukasolson approved these changes Jul 20, 2017

View reviewed changes

Bargs added 9 commits July 21, 2017 09:53

Implement v1 of the Kuery language

7b3bce6

Use ! instead of - as the negation operator

27e0ebc

Update test I left out

3e0db95

Fix filters label for empty query

642ef8d

Add optional format argument to the range function

1409ebd

make sure and/or are always matched without case sensitivity

3f3078b

Clear out after it has been processed

858c73a

ignore invalid query objects

7161e48

set defaults so we don't get undefined errors

b466af5

Bargs force-pushed the enhanceKuery branch from 53fc5e7 to b466af5 Compare July 21, 2017 13:54

Bargs merged commit d379e9a into elastic:master Jul 21, 2017

lukasolson mentioned this pull request Jul 21, 2017

Change default query to match_all #13047

Merged

jimgoodwin added the release_note:roadmap label Jul 25, 2017

This was referenced Jul 28, 2017

Dashboard context returns invalid filter #13181

Closed

Add Checks to Dashboard Context #13182

Merged

Bargs mentioned this pull request Aug 2, 2017

Remove simple_query_string hack now that multi_match supports * properly #13285

Merged

Bargs mentioned this pull request Aug 17, 2017

Build a generic query manipulation API #13552

Closed

trevan mentioned this pull request Aug 17, 2017

Make kuery's use of implicit ANDs more clear #13563

Closed


		describe('toKueryExpression', function () {

		it('should serialize "and" nodes with an implicit syntax by default', function () {


		return function (state) {

		function add(field, values = [], operation, index) {

Implement new Kibana query language #12624

Implement new Kibana query language #12624

Uh oh!

Conversation

Bargs commented Jun 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weltenwort commented Jul 3, 2017

Uh oh!

Bargs commented Jul 3, 2017

Uh oh!

Bargs commented Jul 3, 2017

Uh oh!

weltenwort commented Jul 5, 2017

Uh oh!

dakrone commented Jul 5, 2017

Uh oh!

Bargs commented Jul 5, 2017

Uh oh!

Bargs commented Jul 6, 2017

Uh oh!

weltenwort left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weltenwort Jul 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weltenwort Jul 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weltenwort Jul 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bargs commented Jun 30, 2017 •

edited

Loading

weltenwort Jul 10, 2017 •

edited

Loading

weltenwort Jul 10, 2017 •

edited

Loading

weltenwort Jul 10, 2017 •

edited

Loading