-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add query shape field type #137
Conversation
7137367
to
e89c43d
Compare
...ain/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java
Show resolved
Hide resolved
Signed-off-by: David Zane <davizane@amazon.com>
I am working on the query type changes. Will open a new PR with the changes. Also need to integrate with the settings changes and make sure we can handle field type and field name enabled separately. |
* | ||
* @opensearch.internal | ||
*/ | ||
final class MappingVisitor { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not really MappingVisitor
, since we don't want to visit all the mappings, but rather specific field we are interested in
if (idx == fieldName.length - 1) { | ||
return (String) fieldMapping.get("type"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let us not assume the presence of type
within fieldMapping. This can potentially throw exception
MappingMetadata mappingMetadata = allIndicesMap.get(index.getName()); | ||
if (mappingMetadata != null) { | ||
@SuppressWarnings("unchecked") | ||
Map<String, ?> propertiesAsMap = (Map<String, ?>) mappingMetadata.getSourceAsMap().get("properties"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we preserve this propertiesAsMap
for an index for the search request? Don't really want to create parse the same mappingMetadata for getting different fieldTypes within same index
String fieldType = MappingVisitor.visitMapping(propertiesAsMap, fieldName.split("\\."), 0); | ||
if (fieldType != null) { | ||
// add item to cache | ||
fieldTypeMap.computeIfAbsent(index, k -> new ConcurrentHashMap<>()).put(fieldName, fieldType); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use computeIfAbsent
instead of put
even for the inner method?
Description
Add query shape field type
Issues Resolved
List any issues this PR will resolve, e.g. Closes [...].
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.