SQL: Use field caps inside DESCRIBE TABLE as well #41377
Conversation
Thanks to elastic#34071, there is enough information in field caps to infer the table structure and thus use the same API consistently across the IndexResolver.
|
Pinging @elastic/es-search |
matriv
left a comment
matriv
left a comment
There was a problem hiding this comment.
LGTM. Left some minor comments.
| String name = entry.getKey(); | ||
| for (Entry<String, FieldCapabilities> type : types.entrySet()) { | ||
| // skip unmapped | ||
| if (UNMAPPED.equals(type.getKey())) { |
There was a problem hiding this comment.
Personal preference: I'd inverse the check and surround the whole block with this if, rather than use continue
There was a problem hiding this comment.
I'm not a fan of continue but I prefer it instead of indenting a long method already wrapped...
There was a problem hiding this comment.
Yep, I understand, and I'm not insisting, I didn't comment at all here: https://github.com/elastic/elasticsearch/pull/41377/files/2e890c11f3283d32c6325f7d9ebb7d14b1972ec5#diff-e9f288c0280d1beafe4dc4cf86c12c7bR433 where the block is even larger as inverting the if makes it much less readable.
| return esField; | ||
| } | ||
|
|
||
| // make a copy of the list to be able to modify it | ||
| concreteIndices = new ArrayList<>(asList(capIndices)); | ||
| if (unmappedIndices.isEmpty() == false) { | ||
| concreteIndices.removeAll(unmappedIndices); |
There was a problem hiding this comment.
Maybe not so important: I would use a Set for the unmappedIndices and use contains() to speed up the removal. This way you can iterate on the String[] and create the concreteIndices list.
costin
force-pushed
the
field_caps_migration_2
branch
from
7277e9c to
1853ac3
Compare
Thanks to elastic#34071, there is enough information in field caps to infer the table structure and thus use the same API consistently across the IndexResolver.
5 participants
Thanks to #34071, there is enough information in field caps to infer
the table structure and thus use the same API consistently across the
IndexResolver.