Fix `range` queries on `_type` field for singe type indices #31756

cbuescher · 2018-07-03T10:16:10Z

With the introduction of single types in 6.x, the _type field is no longer
indexed, which leads to certain queries that were working before throw errors
now. One such query is the range query, that, if performed on a single typer
index, currently throws an IAE since the field is not indexed.
This change adds special treatment for this case in the TypeFieldMapper,
comparing the range queries lower and upper bound to the one existing type and
either returns a MatchAllDocs or a MatchNoDocs query.

Relates to #31632
Closes #31476

With the introduction of single types in 6.x, the `_type` field is no longer indexed, which leads to certain queries that were working before throw errors now. One such query is the `range` query, that, if performed on a single typer index, currently throws an IAE since the field is not indexed. This change adds special treatment for this case in the TypeFieldMapper, comparing the range queries lower and upper bound to the one existing type and either returns a MatchAllDocs or a MatchNoDocs query. Relates to elastic#31632 Closes elastic#31476

elasticmachine · 2018-07-03T10:16:13Z

Pinging @elastic/es-search-aggs

cbuescher · 2018-07-03T10:17:07Z

server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java

+                String type = context.getMapperService().documentMapper().type();
+                Query result = new MatchAllDocsQuery();
+                if (lowerTerm != null && lowerTerm instanceof BytesRef) {
+                    int comp = type.compareTo(((BytesRef) lowerTerm).utf8ToString());


Not sure if there is a better way of comparing BytesRef instances, maybe taking into account different lexicographical orderings?

BytesRef implements Comparable, so you should be able to do something like:

int comp = lowerTerm.compareTo(new BytesRef(type));

cbuescher · 2018-07-03T10:18:32Z

server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java

+            } else {
+                // this means the index has a single type and the type field is implicit
+                DEPRECATION_LOGGER.deprecatedAndMaybeLog("range_single_type",
+                        "Running [range] query on [_type] field for an index with a single type. As types are deprecated, this functionality will be removed in future releases.");


Not sure if we really need the deprecation, but since it was mentioned at some point I added it for discussion purposes.

jpountz · 2018-07-03T11:53:49Z

server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java

@@ -154,6 +159,33 @@ public Query termsQuery(List<?> values, QueryShardContext context) {
            }
        }

+        @Override
+        public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, QueryShardContext context) {
+            if (hasDocValues()) {


we don't need this if block, do we? All 6.x and 7.x indices have a single type.

Don't we still support multi-type indices that have been created in 5.x on 6.x? I will remove this for the PR against master but I think we need it for 6.x in that case. Can you confirm?

jpountz · 2018-07-03T11:58:56Z

server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java

+                        "Running [range] query on [_type] field for an index with a single type. As types are deprecated, this functionality will be removed in future releases.");
+                String type = context.getMapperService().documentMapper().type();
+                Query result = new MatchAllDocsQuery();
+                if (lowerTerm != null && lowerTerm instanceof BytesRef) {


we shouldn't be lenient in case lowerTerm doesnt't implement BytesRef

I was wondering the same, do we want to throw an IAE in those cases?

I would call indexedValueForSearch.

jpountz · 2018-07-03T11:59:23Z

server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java

+                        result = new MatchNoDocsQuery("[_type] was less then lower bound of range");
+                    }
+                }
+                if (upperTerm != null && upperTerm instanceof BytesRef) {


we shouldn't be lenient in case upperTerm doesnt't implement BytesRef

cbuescher · 2018-07-03T19:27:12Z

@romseygeek @jpountz thanks, I updated the issue, left one question about wether we need to still support multi-type indices on 6.x (the ones created in 5) in case we backport this.

romseygeek · 2018-07-04T08:01:35Z

LGTM

cbuescher · 2018-07-04T12:17:43Z

@jpountz are you also okay with this change now? Still waiting for green CI to merge, but I left one question about backporting to 6.4. In that case we still need the support for multi-type indices that I deleted in this PR against master, no?

jpountz · 2018-07-17T13:50:05Z

server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java

+            }
+            if ((upperTerm != null && upperTerm instanceof BytesRef == false)) {
+                throw new IllegalArgumentException("upper term should be BytesRef but was: " + upperTerm.getClass().getName());
+            }


Conversion to bytesref is done elsewhere with indexedValueForSearch. I'm unsure of the impact of rejecting anything but bytesrefs.

…31756) With the introduction of single types in 6.x, the `_type` field is no longer indexed, which leads to certain queries that were working before throw errors now. One such query is the `range` query, that, if performed on a single typer index, currently throws an IAE since the field is not indexed. This change adds special treatment for this case in the TypeFieldMapper, comparing the range queries lower and upper bound to the one existing type and either returns a MatchAllDocs or a MatchNoDocs query. Relates to elastic#31632 Closes elastic#31476

@return

* master: Painless: Simplify Naming in Lookup Package (#32177) Handle missing values in painless (#32207) add support for write index resolution when creating/updating documents (#31520) ECS Task IAM profile credentials ignored in repository-s3 plugin (#31864) Remove indication of future multi-homing support (#32187) Rest test - allow for snapshots to take 0 milliseconds Make x-pack-core generate a pom file Rest HL client: Add put watch action (#32026) Build: Remove pom generation for plugin zip files (#32180) Fix comments causing errors with Java 11 Fix rollup on date fields that don't support epoch_millis (#31890) Detect and prevent configuration that triggers a Gradle bug (#31912) [test] port linux package packaging tests (#31943) Revert "Introduce a Hashing Processor (#31087)" (#32178) Remove empty @return from JavaDoc Adjust SSLDriver behavior for JDK11 changes (#32145) [test] use randomized runner in packaging tests (#32109) Add support for field aliases. (#32172) Painless: Fix caching bug and clean up addPainlessClass. (#32142) Call setReferences() on custom referring tokenfilters in _analyze (#32157) Fix BwC Tests looking for UUID Pre 6.4 (#32158) Improve docs for search preferences (#32159) use before instead of onOrBefore Add more contexts to painless execute api (#30511) Add EC2 credential test for repository-s3 (#31918) A replica can be promoted and started in one cluster state update (#32042) Fix Java 11 javadoc compile problem Fix CP for namingConventions when gradle home has spaces (#31914) Fix `range` queries on `_type` field for singe type indices (#31756) [DOCS] Update TLS on Docker for 6.3 (#32114) ESIndexLevelReplicationTestCase doesn't support replicated failures but it's good to know what they are Remove versionType from translog (#31945) Switch distribution to new style Requests (#30595) Build: Skip jar tests if jar disabled Painless: Add PainlessClassBuilder (#32141) Build: Make additional test deps of check (#32015) Disable C2 from using AVX-512 on JDK 10 (#32138) Build: Move shadow customizations into common code (#32014) Painless: Fix Bug with Duplicate PainlessClasses (#32110) Remove empty @param from Javadoc Re-disable packaging tests on suse boxes Docs: Fix missing example script quote (#32010) [ML] Wait for aliases in multi-node tests (#32086) [ML] Move analyzer dependencies out of categorization config (#32123) Ensure to release translog snapshot in primary-replica resync (#32045) Handle TokenizerFactory TODOs (#32063) Relax TermVectors API to work with textual fields other than TextFieldType (#31915) Updates the build to gradle 4.9 (#32087) Mute :qa:mixed-cluster indices.stats/10_index/Index - all’ Check that client methods match API defined in the REST spec (#31825) Enable testing in FIPS140 JVM (#31666) Fix put mappings java API documentation (#31955) Add exclusion option to `keep_types` token filter (#32012) [Test] Modify assert statement for ssl handshake (#32072)

…32161) With the introduction of single types in 6.x, the `_type` field is no longer indexed, which leads to certain queries that were working before throw errors now. One such query is the `range` query, that, if performed on a single typer index, currently throws an IAE since the field is not indexed. This change adds special treatment for this case in the TypeFieldMapper, comparing the range queries lower and upper bound to the one existing type and either returns a MatchAllDocs or a MatchNoDocs query. Relates to #31632 Closes #31476

* 6.x: Security: revert to old way of merging automata (#32254) Fix a test bug in RangeQueryBuilderTests introduced in the field aliases backport. Introduce Application Privileges with support for Kibana RBAC (#32309) Undo a debugging change that snuck in during the field aliases merge. [test] port linux package packaging tests (#31943) Painless: Update More Methods to New Naming Scheme (#32305) Tribe: Add error with secure settings copied to tribe (#32298) Add V_6_3_3 version constant Add ERR to ranking evaluation documentation (#32314) [DOCS] Added link to 6.3.2 RNs [DOCS] Updates 6.3.2 release notes with PRs from ml-cpp repo (#32334) [Kerberos] Add Kerberos authentication support (#32263) [ML] Extract persistent task methods from MlMetadata (#32319) Backport - Add Snapshots Status API to High Level Rest Client (#32295) Make release notes ignore the `>test-failure` label. (#31309) [DOCS] Adds release highlights for search for 6.4 (#32095) Allow Integ Tests to run in a FIPS-140 JVM (#32316) Add support for field aliases to 6.x. (#32184) Register ERR metric with NamedXContentRegistry (#32320) fixes broken build for third-party-tests (#32315) Relates #31918 / Closes infra/issues/6085 [DOCS] Rollup Caps API incorrectly mentions GET Jobs API (#32280) Rest HL client: Add put watch action (#32026) (#32191) Add WeightedAvg metric aggregation (#31037) Consistent encoder names (#29492) Switch monitoring to new style Requests (#32255) specify subdirs of lib, bin, modules in package (#32253) Rename ranking evaluation `quality_level` to `metric_score` (#32168) Add new permission for JDK11 to load JAAS libraries (#32132) Switch x-pack:core to new style Requests (#32252) Watcher: Store username on watch execution (#31873) Silence SSL reload test that fails on JDK 11 Painless: Clean up add methods in PainlessLookup (#32258) CCE when re-throwing "shard not available" exception in TransportShardMultiGetAction (#32185) Fail shard if IndexShard#storeStats runs into an IOException (#32241) Fix `range` queries on `_type` field for singe type indices (#31756) (#32161) AwaitsFix RecoveryIT#testHistoryUUIDIsGenerated Add new fields to monitoring template for Beats state (#32085) (#32273) [TEST] improve REST high-level client naming conventions check (#32244) Check that client methods match API defined in the REST spec (#31825)

cbuescher added >bug review :Search Foundations/Mapping Index mappings, including merging and defining field types v7.0.0 v6.4.0 labels Jul 3, 2018

cbuescher requested a review from jpountz July 3, 2018 10:16

cbuescher commented Jul 3, 2018

View reviewed changes

jpountz requested changes Jul 3, 2018

View reviewed changes

iter

04b2d9d

Merge branch 'master' into rangeQuery-singleType

f9f392b

jpountz approved these changes Jul 17, 2018

View reviewed changes

Christoph Büscher added 2 commits July 17, 2018 15:53

Merge branch 'master' into rangeQuery-singleType

05e3001

iter

c5f698a

jpountz approved these changes Jul 17, 2018

View reviewed changes

cbuescher merged commit 15f95a9 into elastic:master Jul 18, 2018

cbuescher mentioned this pull request Jul 18, 2018

Fix range queries on _type field for singe type indices (#31756) #32161

Merged

cbuescher added the backport pending label Jul 18, 2018

cbuescher removed the backport pending label Jul 23, 2018

colings86 added the v7.0.0-beta1 label Feb 7, 2019

colings86 removed the v7.0.0 label Feb 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `range` queries on `_type` field for singe type indices #31756

Fix `range` queries on `_type` field for singe type indices #31756

cbuescher commented Jul 3, 2018

elasticmachine commented Jul 3, 2018

cbuescher Jul 3, 2018

romseygeek Jul 3, 2018

cbuescher Jul 3, 2018

jpountz Jul 3, 2018

cbuescher Jul 3, 2018

jpountz Jul 17, 2018

jpountz Jul 3, 2018

cbuescher Jul 3, 2018

jpountz Jul 17, 2018

jpountz Jul 3, 2018

cbuescher commented Jul 3, 2018

romseygeek commented Jul 4, 2018

cbuescher commented Jul 4, 2018

jpountz Jul 17, 2018

Fix range queries on _type field for singe type indices #31756

Fix range queries on _type field for singe type indices #31756

Conversation

cbuescher commented Jul 3, 2018

elasticmachine commented Jul 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbuescher commented Jul 3, 2018

romseygeek commented Jul 4, 2018

cbuescher commented Jul 4, 2018

Choose a reason for hiding this comment

Fix `range` queries on `_type` field for singe type indices #31756

Fix `range` queries on `_type` field for singe type indices #31756