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

Strict query parsing on percolators and alias filters #6664

Closed
clintongormley opened this issue Jul 1, 2014 · 12 comments
Closed

Strict query parsing on percolators and alias filters #6664

clintongormley opened this issue Jul 1, 2014 · 12 comments

Comments

@clintongormley
Copy link

Today, if we create a filtered alias or a percolator query which references a field that is not yet mapped, the field is interpreted as a string.

This is a problem when (eg) the user then indexes a document where the field is really numeric or a date. Any existing filters or percolator queries will not work correctly until the cached filter/query is regenerated (eg after a restart).

Instead, we should support a "strict" query parsing mode which throws an error if an unmapped field is referenced.

@clintongormley
Copy link
Author

See #6572

@jpountz
Copy link
Contributor

jpountz commented Jul 4, 2014

One option might be to make the filter evaluation lazy until fields are mapped but that would add significant complexity/overhead so I think strict query parsing would be the way to go! Maybe we could have a helper method on the context to resolve fields that would return null when strict parsing is disabled and raise an error otherwise?

@clintongormley clintongormley changed the title Support strict query parsing Strict query parsing on percolators and alias filters Jul 4, 2014
@clintongormley
Copy link
Author

Also see #6110 - term lookup filters should verify that the doc actually exists when the filter is instantiated.

@s1monw
Copy link
Contributor

s1monw commented Sep 10, 2014

@martijnvg did this not get closed or is it only partially resolved ? Anyway can we close or push to 1.5?

@martijnvg
Copy link
Member

@s1monw The strict field resolution is already pushed. I forgot that there was an issue for this!

@clintongormley Regarding the terms lookup filter, the #6928 PR doesn't cover this since it only takes care of unmapped fields in the mapping. I think we should open a different issue for this? Also the geo_shape filter with is referring to an indexed shape suffers from the same problem.

@s1monw
Copy link
Contributor

s1monw commented Sep 10, 2014

@martijnvg can we close it?

@martijnvg
Copy link
Member

@clintongormley Ow wait, the geo_shape filters fail if an indexed shape document doesn't exist. I think we should do the same for the terms lookup filter? (right now it silently ignores it by not matching anything)

@martijnvg
Copy link
Member

@s1monw yes, imo we can close this.

@andreasch
Copy link

Shouldn't alias filters at least allow referencing fields that are internal like _type, _id or _routing?

@clintongormley
Copy link
Author

@andreasch this sounds like it should be a new issue?

@andreasch
Copy link

@clintongormle I believe it is a new issue and I can go create one if it makes sense.

We have a multi-tenant implementation where the routing value is equal to the tenant name and routing is always required and used.

We also use alias filtering as described here:
http://www.elastic.co/guide/en/elasticsearch/guide/master/faking-it.html

For our alias filters we have something like this:

{
    "term":{
        "_routing":"tenantName"
    }
}

However, Elasticsearch versions 1.4 and after do not allow for such an alias filter to be created.

@clintongormley
Copy link
Author

@andreasch ok - I thought you meant something else. You can create filters like that today, but you need to set the _routing field to be indexed. (This will be done by default in v2). See http://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-routing-field.html#_store_index

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants