Skip to content

Conversation

@jtibshirani
Copy link
Contributor

No description provided.

@jtibshirani jtibshirani added >bug WIP :Search Foundations/Mapping Index mappings, including merging and defining field types v7.0.0 v6.5.0 labels Aug 21, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@jtibshirani jtibshirani force-pushed the exists-queries-on-keywords branch from a15b142 to 2be290d Compare August 21, 2018 04:11
Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks @jtibshirani
Should we also disable the creation of the field names field if norms are activated:

} else if (fieldType().stored() || fieldType().indexOptions() != IndexOptions.NONE) {
?

@jtibshirani jtibshirani removed the WIP label Aug 21, 2018
@jtibshirani jtibshirani force-pushed the exists-queries-on-keywords branch from ef4cc72 to 5b9e478 Compare August 21, 2018 20:40
@jtibshirani jtibshirani force-pushed the exists-queries-on-keywords branch from 5b9e478 to b03e5ea Compare August 21, 2018 20:42
Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a minor comment, LGTM otherwise.

Field field = new Field(fieldType().name(), binaryValue, fieldType());
fields.add(field);

if (!fieldType().hasDocValues() && fieldType().omitNorms()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we prefer to use fieldType().hasDocValues() == false for clarity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. I will struggle a bit with this inside but codebase consistency is most important :)

@jtibshirani jtibshirani merged commit 67b5a83 into elastic:master Aug 21, 2018
@jtibshirani jtibshirani deleted the exists-queries-on-keywords branch August 21, 2018 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Search Foundations/Mapping Index mappings, including merging and defining field types v6.4.1 v6.5.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants