Skip to content

Make keyword a family of field types#58315

Merged
markharwood merged 5 commits intoelastic:masterfrom
markharwood:fix/53175
Jun 24, 2020
Merged

Make keyword a family of field types#58315
markharwood merged 5 commits intoelastic:masterfrom
markharwood:fix/53175

Conversation

@markharwood
Copy link
Copy Markdown
Contributor

@markharwood markharwood commented Jun 18, 2020 •
edited
Loading

This change introduces a familyTypeName method to MappedFieldType for use in the field capabilities API.
The constant_keyword and wildcard fields override this to return keyword to describe the family to which they belong.

Relates to #53175

@markharwood markharwood added the :Search Foundations/Mapping Index mappings, including merging and defining field types label Jun 18, 2020
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Jun 18, 2020

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

@elasticmachine elasticmachine added the Team:Search Meta label for search team label Jun 18, 2020
@markharwood markharwood self-assigned this Jun 18, 2020
@markharwood
Copy link
Copy Markdown
Contributor Author

markharwood commented Jun 18, 2020

@costin - do you have any pointers for the best place to reset the SQL expectations that are failing here (see test output) now that I've changed the field capabilities as per what we agreed in #53175 ?

@jimczi
Copy link
Copy Markdown
Contributor

jimczi commented Jun 18, 2020

do you have any pointers for the best place to reset the SQL expectations that are failing here (see test output) now that I've changed the field capabilities as per what we agreed in #53175 ?

I guess we have no choice but to replace all instance of constant_keyword by keyword in SQL. It seems that most of the errors are on the test expectation that should be changed to keyword or KEYWORD depending on the context ?

@markharwood
Copy link
Copy Markdown
Contributor Author

markharwood commented Jun 18, 2020

I got a little lost chasing expected vs actuals discrepancies when it looks like a lot of the expectations might come from *csv-spec files.

@costin
Copy link
Copy Markdown
Member

costin commented Jun 18, 2020

@markharwood you're on the right path. The *.csv-spec files are the ones that indicate what we expect. For example:
org.elasticsearch.xpack.sql.qa.security.JdbcCsvSpecIT.test {alias.testDescribeAlias} indicates there's a failure inside alias.csv-spec for describeAlias, namely here.

Hope this helps.

@costin
Copy link
Copy Markdown
Member

costin commented Jun 18, 2020

Tagging @astefan for awareness since once this PR lands, we should remove the constant_keyword handling.

This was referenced Jun 18, 2020
Closed
Open
@costin costin mentioned this pull request Jun 18, 2020
Merged
@markharwood markharwood force-pushed the fix/53175 branch 2 times, most recently from 2bbc75d to 0609e98 Compare June 23, 2020 11:24
@markharwood markharwood removed the WIP label Jun 23, 2020
@markharwood markharwood requested review from astefan and jimczi June 23, 2020 12:23
astefan
astefan approved these changes Jun 24, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

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

LGTM

@astefan astefan mentioned this pull request Jun 24, 2020
Closed
jimczi
jimczi approved these changes Jun 24, 2020
View reviewed changes
Copy link
Copy Markdown
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.

LGTM, we should also update the documentation in _field_caps to explain the keyword family ?

@markharwood markharwood merged commit cdc1be1 into elastic:master Jun 24, 2020
markharwood added a commit to markharwood/elasticsearch that referenced this pull request Jun 24, 2020
@markharwood
Field capabilities - make keyword a family of field types (elastic#…
a9350f3 
…58315)

Introduces a new method on `MappedFieldType` to return a family type name which defaults to the field type.
Changes `wildcard` and `constant_keyword` field types to return `keyword` for field capabilities.

Relates to elastic#53175
markharwood added a commit that referenced this pull request Jun 24, 2020
@markharwood
Field capabilities - make keyword a family of field types (#58315) (#…
d5ac3bb 
…58483)

Introduces a new method on `MappedFieldType` to return a family type name which defaults to the field type.
Changes `wildcard` and `constant_keyword` field types to return `keyword` for field capabilities.

Relates to #53175
@jtibshirani
Copy link
Copy Markdown
Contributor

jtibshirani commented Jun 24, 2020

@markharwood perhaps we should make a note in the migration/ breaking changes docs for 7.x explaining that constant_keyword is now presented as keyword.

@russcam russcam mentioned this pull request Jul 23, 2020
Closed
29 tasks
@astefan astefan mentioned this pull request Jul 31, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@astefan astefan astefan approved these changes

@jimczi jimczi jimczi approved these changes

Assignees

@markharwood markharwood

Labels

>enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Meta label for search team v7.9.0 v8.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@markharwood @elasticmachine @jimczi @costin @jtibshirani @astefan @jakelandis