Skip to content

Conversation

@jpountz
Copy link
Contributor

@jpountz jpountz commented Sep 8, 2017

Today we have all non-plugin mappers in core. I'd like to start moving those
that neither map to json datatypes nor are very frequently used like date or
ip to a module.

This commit creates a new module called mappers-extra and moves the
scaled_float and token_count mappers to it. I'd like to eventually move
range fields there but it's more complicated due to their intimate
relationship with range queries.

Relates #10368

@jpountz jpountz added :Search Foundations/Mapping Index mappings, including merging and defining field types >non-issue v6.1.0 v7.0.0 labels Sep 8, 2017
@rpedela
Copy link

rpedela commented Sep 11, 2017

I have used date in almost every single index I have created over the last ~5 years using ES. I often need to sort by date and/or time, and I highly doubt I am the only one. date support seems like it should be a core feature.

@dadoonet
Copy link
Contributor

Don't be afraid. Moving to modules does not mean that mappers are not available anymore by default. Modules are like mandatory plugins. You will not see the difference.

@jpountz
Copy link
Contributor Author

jpountz commented Sep 11, 2017

What David says is correct, there won't be any differences for end users. Also the wording was confusing but date and ip are staying in core. token_count and scaled_float are moving.

Today we have all non-plugin mappers in core. I'd like to start moving those
that neither map to json datatypes nor are very frequently used like `date` or
`ip` to a module.

This commit creates a new module called `mappers-extra` and moves the
`scaled_float` and `token_count` mappers to it. I'd like to eventually move
`range` fields there but it's more complicated due to their intimate
relationship with range queries.

Relates elastic#10368
Copy link
Member

Choose a reason for hiding this comment

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

This should be possible to have pluggable, yeah.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is accurate. At least, in the other bwc builds there is a lot of gradle work to set these up and it isn't here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

copy-paste, I'll fix

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it makes more sense to put this in the mapper extra module. Are you doing this because you know it'll need some other module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know but this is a possibility indeed.

Copy link
Member

Choose a reason for hiding this comment

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

A few of us have been talking lately about trying to consolidate the qa modules rather than the one for this, one for that style that we have now. Given that desire I think it'd be better to move this into the module. If we have to move it back to a qa module later we can find it a better home then. Hopefully we won't though.

@jpountz jpountz force-pushed the modules/mapper_extras branch from 10204d5 to fa62a87 Compare September 11, 2017 14:10
@jpountz
Copy link
Contributor Author

jpountz commented Sep 11, 2017

Thanks for having a look @nik9000 . Thanks to #26552 I was able to move range fields too. Don't be too scared of the number of added lines, this is mostly because I added REST tests.

@jpountz
Copy link
Contributor Author

jpountz commented Sep 12, 2017

@nik9000 I removed the qa module.

@Override
protected final boolean isEqual(Range o) {
AbstractRange other = (AbstractRange) o;
AbstractRange<?> other = (AbstractRange<?>) o;
Copy link
Member

Choose a reason for hiding this comment

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

❤️

jpountz and others added 5 commits September 13, 2017 10:05
* master: (21 commits)
  Ensure module is bundled before installing in tests
  Add boolean similarity to built in similarity types (elastic#26613)
  [Tests] Remove skip tests in search/30_limits.yml
  Let search phases override max concurrent requests
  Add a soft limit for the number of requested doc-value fields (elastic#26574)
  Support for accessing Azure repositories through a proxy (elastic#23518)
  Add beta tag to MSI Windows Installer (elastic#26616)
  Fix Lucene version of 5.6.1.
  Remove azure deprecated settings (elastic#26099)
  Handle the 5.6.0 release
  Allow plugins to validate cluster-state on join (elastic#26595)
  Remove index mapper dynamic settings (elastic#25734)
  update AWS SDK for ECS Task IAM support in discovery-ec2 (elastic#26479)
  Azure repository: Accelerate the listing of files (used in delete snapshot) (elastic#25710)
  Build: Remove norelease from forbidden patterns (elastic#26592)
  Fix reference to painless inside expression engine (elastic#26528)
  Build: Move javadoc linking to root build.gradle (elastic#26529)
  Test: Remove leftover static bwc test case (elastic#26584)
  Docs: Remove remaining references to file and native scripts (elastic#26580)
  Snapshot fallback should consider build.snapshot
  ...
@jpountz jpountz merged commit 93da772 into elastic:master Sep 13, 2017
@jpountz jpountz deleted the modules/mapper_extras branch September 13, 2017 15:59
jasontedor added a commit to synhershko/elasticsearch that referenced this pull request Sep 14, 2017
* master: (39 commits)
  [Docs] Correct typo in removal_of_types.asciidoc (elastic#26646)
  [Docs] "The the" is a great band, but ... (elastic#26644)
  Move all repository-azure classes under one single package (elastic#26624)
  [Docs] Update link in removal_of_types.asciidoc (elastic#26614)
  Fix percolator highlight sub fetch phase to not highlight query twice (elastic#26622)
  Refactor bootstrap check results and error messages
  Add BootstrapContext to expose settings and recovered state to bootstrap checks (elastic#26628)
  [Tests] Removing skipping tests in search rest tests
  Initialize checkpoint tracker with allocation ID
  Move non-core mappers to a module. (elastic#26549)
  [Docs] Clarify size parameter in Completion Suggester doc (elastic#26617)
  Add soft limit on allowed number of script fields in request (elastic#26598)
  Remove MapperService#dynamic. (elastic#26603)
  Fix incomplete sentences in parent-join docs (elastic#26623)
  More efficient encoding of range fields. (elastic#26470)
  Ensure module is bundled before installing in tests
  Add boolean similarity to built in similarity types (elastic#26613)
  [Tests] Remove skip tests in search/30_limits.yml
  Let search phases override max concurrent requests
  Add a soft limit for the number of requested doc-value fields (elastic#26574)
  ...
jpountz added a commit that referenced this pull request Sep 15, 2017
Today we have all non-plugin mappers in core. I'd like to start moving those
that neither map to json datatypes nor are very frequently used like `date` or
`ip` to a module.

This commit creates a new module called `mappers-extra` and moves the
`scaled_float` and `token_count` mappers to it. I'd like to eventually move
`range` fields there but it's more complicated due to their intimate
relationship with range queries.

Relates #10368
cbuescher pushed a commit that referenced this pull request Jan 23, 2018
…der tests (#28171)

The tests for those field types were removed in #26549 because the range mapper
was moved to a module, but later this mapper was moved back to core in #27854.
This change adds back those two field types like before to the general setup in
AbstractQueryTestCase and adds some specifics to the RangeQueryBuilder and
TermsQueryBuilder tests. Also adding back an integration test in SearchQueryIT that
has been removed before but that can be kept with the mapper back in core now.

Relates to #28147
cbuescher pushed a commit that referenced this pull request Jan 23, 2018
…der tests (#28171)

The tests for those field types were removed in #26549 because the range mapper
was moved to a module, but later this mapper was moved back to core in #27854.
This change adds back those two field types like before to the general setup in
AbstractQueryTestCase and adds some specifics to the RangeQueryBuilder and
TermsQueryBuilder tests. Also adding back an integration test in SearchQueryIT that
has been removed before but that can be kept with the mapper back in core now.

Relates to #28147
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>non-issue :Search Foundations/Mapping Index mappings, including merging and defining field types v6.1.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants