Unified MetaData#concreteIndices methods into a single method that accepts indices (or aliases) and indices options

[javanna]

The goal of this PR is to have a single method in MetaData that allows to resolve aliases or indices (eventually containing wildcard expressions) to concrete indices. That way all the apis will just refer to this same new method passing the proper indices options as argument.

In order to achieve that a new internal flag has been added to IndicesOptions, which tells whether aliases can be resolved to multiple indices or not.

Also it's now possible to decide not to expand wildcards at all via indices options, while previously it was possible to decide whether to expand them to open indices, closed indices, or both.

Note that the PR is against 1.x to handle backwards compatibility properly when needed. Bw comp checks will be removed when porting to master.

[uboness]

First thing first... lets move all the flags to named constants... cause it's really becoming hard to follow

[uboness]

Left a few comments, overall great direction. While you're at it, would be nice to have a builder for the IndexOptions (relieve all parsers from keeping all these bool flags around)

[javanna]

@uboness wondering if we still need this optimization, removing doesn't change anything, does it really help performance wise???

[uboness]

seems redundant indeed

[javanna]

I just pushed a few more commits that address the comments and finalize the refactoring.

[s1monw]

should this be a real exception? I mean it's clearly wrong?

[uboness]

yeah... agree... an assert does't make much sense here... in fact, I'd consider removing this method all together and move the check to the api that requires a single index (so the exception message will have the right context)

[javanna]

both good points... the reason why it's an assert is that it should never happen, as we pass in a single index and we say we don't resolve aliases to multiple indices. Can make it an exception but then it would seem that we handle the case while this must not happen unless there are bugs.

As for removing the method entirely. I like the idea, but then we'd end up with these same two lines all over the place...not sure it's great, I thought it's better to share those lines (length check and array to single string conversion).

[uboness]

both good points... the reason why it's an assert is that it should never happen, as we pass in a single index and we say we don't resolve aliases to multiple indices.

but an alias to a single index is allowed? If "yes", then it needs to be an exception, as it's not under the code control what input the user provides (the user can provide an alias that points to multiple indices... )

As for removing the method entirely. I like the idea, but then we'd end up with these same two lines all over the place...not sure it's great, I thought it's better to share those lines (length check and array to single string conversion).

fair point... if it's called all over the place, then perhaps the exception should be thrown here (with somewhat of a generic yet descriptive & user friendly message)

[javanna]

yes an alias to a single index is allowed. but I wasn't too clear... when I said we don't resolve aliases to multiple indices (with these indices options) I meant that we throw an exception in that case! Thus the case where length != 1 never happens whatever the user input is...cause we throw an exception in all other cases with those specific indices options.

[uboness]

ah.. ok, gotcha... I'd leave the assert here but change it to

assert indices.length == 1 : "expected an exception to be thrown otherwise";

for clarity

[s1monw]

LGTM

[s1monw]

I think we should do this differently we want to flip all the bits to 0 that are > 4th bit you should do id & 0xF

[javanna]

Right, if we were to add other flags, the XOR would only flip the 5th bit...

[javanna]

Merged