-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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 json parsing improvements #11996
Comments
+1 to this. Personally I think it should be one setting for the search request in general since I don't see a reason why you would want to be notified of deprecated keys in your queries but not in your aggregations, etc. It would be great to collate the error and print them all at once as this would save users lots of time when debugging syntax problems |
Actually, that setting was added for percolator queries and alias filters, where creating a query on an unmapped field led to it assuming the field was a string. i think the setting was added because a user complained about the change. Honestly, what they are doing is broken and I think we shouldn't support their particular use case. I do wonder if the need for this has been fixed by #11930? (at least for aliases if not for percolators?) I think strict field lookups is distinct from warning/throwing exceptions for the use of deprecated functionality. This issue deals with fieldname lookups: #12016 But yes, I'm in favour of having a setting that controls whether the use of deprecated functionality throws an exception, warns to the deprecation log, or is ignored (see #8963 for the original suggestion) |
Thanks for pointing this out @clintongormley I had a look at the codebase and this is what I found. I do see in the history that The error message that we are used to in the context of aliases and percolation "Strict field resolution and no field mapping can be found for the field with name..." depends on the value of the I think it is worth at this point to introduce a new (more generic) setting to enable deprecation exceptions, which is not just a boolean but allows to either throw exception or log deprecation warning by using the deprecation logger that we now have. |
The The The IMO all these settings are completely obsolete when #12016 gets added and the |
Ah ok - i got the two mixed up.
I'm not sure I agree completely here (but maybe I'm missing something). As I understand it:
|
Ok, lets disable the strict parsing for alias filters.
True, we still need that, I made my conclusions too quick.
is there a bug with |
I opened #12150 for alias filters. |
Not that there is a bug, just that their use case "assume all unmapped fields are strings" seems pretty broken. Why would you create a percolator for an unknown field? (While we're on it, a wildcarded field name in a percolator query suffers from a similar problem: it would just take into account fields that exist when the query is instantiated) |
We currently support the
index.query.parse.strict
setting that allows to turn on strict parsing, so that when deprecated keys are used elasticsearch returns an error. This was thought initialy for queries but got also expanded to anywhere we have json. Unfortunately the name of the setting doesn't reflect what it does anymore, as it can be used in the context of snapshots, aggregations, scripts etc. Let's discuss whether we should rename this setting of have separate settings for each feature. What do people think about this?Also, I think it would be nice to improve this feature by collecting different errors and printing them all out at the same time rather than throwing exception at the first problem found. Or maybe add the possibility to enable logging deprecation usages rather than throwing exception (both can still be supported). Thoughts?
The text was updated successfully, but these errors were encountered: