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

Should we allow fields with * in their name?! #76874

Open
nik9000 opened this issue Aug 24, 2021 · 6 comments
Open

Should we allow fields with * in their name?! #76874

nik9000 opened this issue Aug 24, 2021 · 6 comments
Labels
>bug :Search/Mapping Index mappings, including merging and defining field types Team:Search Meta label for search team

Comments

@nik9000
Copy link
Member

nik9000 commented Aug 24, 2021

Should we let field names contain a *? This gets confusing when you send the fields to us because we usually interpret * as a glob pattern.

Story:
I'm helping some folks debug a fun issue where their kibana discover tab stops working. It looks like they have fields with * in their name. Say c*g.

Kibana, entirely reasonably, reads those fields from field caps and send them back in the fields section of the search request. So:

"fields": [
  { "field": "c*g", "format": "date_time"}
]

We then, helpfully, interpret the * as a glob pattern. Chaos! In the fake example above we'd match the original c*g field but we also match cat.dog and condor.clog. And, sadly, those are not dates! So we throw back an IllegalArgumentException.

@nik9000 nik9000 added >bug :Search/Mapping Index mappings, including merging and defining field types team-discuss labels Aug 24, 2021
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Aug 24, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

@mattkime
Copy link

mattkime commented Aug 26, 2021

I guess I'm curious to hear an argument for supporting * in field names. I think it would be possible to support it in kibana but it would be unlikely to be worth the effort as we'd need to distinguish whether the user wants * the character or * the glob pattern.


Based on what @timroes has to say here - https://github.com/elastic/sdh-elasticsearch/issues/4800#issuecomment-905427386 - they should just be disallowed.

@nik9000
Copy link
Member Author

nik9000 commented Aug 26, 2021 via email

@mattkime
Copy link

Where does this stand? Seems like we should be ready to make a call on this.

@nik9000
Copy link
Member Author

nik9000 commented Sep 11, 2021 via email

@jimczi
Copy link
Contributor

jimczi commented Sep 13, 2021

We discussed this offline and we're not convinced that we should make a call now. The main point of the dynamic mapping is to accept all field names. We could reserve some characters and make them invalid but that doesn't solve the problem. Users would still need to deal with these characters somehow.
IMO, this issue reveals two things:

  • We don't have any mechanism to escape characters in APIs that accept field names and patterns.
  • We shouldn't fail the entire shard request when a single field fails the fields option parser.

None of these requires a breaking change for our users and can be dealt in the general case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Mapping Index mappings, including merging and defining field types Team:Search Meta label for search team
Projects
None yet
Development

No branches or pull requests

4 participants