Skip to content

Commit

Permalink
Switch from using docvalue_fields to extracting values from _source (e…
Browse files Browse the repository at this point in the history
…lastic#44062)

* Switch from using docvalue_fields to extracting values from _source
where applicable. Doing this means parsing the _source and handling the
numbers parsing just like Elasticsearch is doing it when it's indexing
a document.
* This also introduces a minor limitation: aliases type of fields that
are NOT part of a tree of sub-fields will not be able to be retrieved
anymore. field_caps API doesn't shed any light into a field being an
alias or not and at _source parsing time there is no way to know if a
root field is an alias or not. Fields of the type "a.b.c.alias" can be
extracted from docvalue_fields, only if the field they point to can be
extracted from docvalue_fields. Also, not all fields in a hierarchy of
fields can be evaluated to being an alias.

(cherry picked from commit 8bf8a05)
  • Loading branch information
astefan committed Jul 24, 2019
1 parent c329b45 commit 74172ad
Show file tree
Hide file tree
Showing 23 changed files with 1,118 additions and 71 deletions.
6 changes: 2 additions & 4 deletions docs/reference/sql/endpoints/translate.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ Which returns:
{
"size" : 10,
"docvalue_fields" : [
{
"field": "page_count"
},
{
"field": "release_date",
"format": "epoch_millis"
Expand All @@ -35,7 +32,8 @@ Which returns:
"_source": {
"includes": [
"author",
"name"
"name",
"page_count"
],
"excludes": []
},
Expand Down
18 changes: 18 additions & 0 deletions docs/reference/sql/limitations.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -161,3 +161,21 @@ By default,`geo_points` fields are indexed and have doc values. However only lat
indexed with some loss of precision from the original values (4.190951585769653E-8 for the latitude and
8.381903171539307E-8 for longitude). The altitude component is accepted but not stored in doc values nor indexed.
Therefore calling `ST_Z` function in the filtering, grouping or sorting will return `null`.

[float]
[[fields-from-source]]
=== Retrieving from `_source`

Most of {es-sql}'s columns are retrieved from the document's `_source` and there is no attempt to get the columns content from
`docvalue_fields` not even in the case <<mapping-source-field,`_source`>> field is disabled in the mapping explicitly.
If a column, for which there is no source stored, is asked for in a query, {es-sql} will not return it. Field types that don't follow
this restriction are: `keyword`, `date`, `scaled_float`, `geo_point`, `geo_shape` since they are NOT returned from `_source` but
from `docvalue_fields`.

[float]
[[fields-from-docvalues]]
=== Retrieving from `docvalue_fields`

When the number of columns retrieveable from `docvalue_fields` is greater than the configured <<dynamic-index-settings,`index.max_docvalue_fields_search` setting>>
the query will fail with `IllegalArgumentException: Trying to retrieve too many docvalue_fields` error. Either the mentioned {es}
setting needs to be adjusted or fewer columns retrieveable from `docvalue_fields` need to be selected.
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ public void compare(String field, @Nullable Object actual, Object expected) {
field(field, "same [" + expected + "]");
return;
}
field(field, "expected [" + expected + "] but was [" + actual + "]");
field(field, "expected " + expected.getClass().getSimpleName() + " [" + expected + "] but was "
+ actual.getClass().getSimpleName() + " [" + actual + "]");
}

private void indent() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public void testAssertXContentEquivalentErrors() throws IOException {
AssertionError error = expectThrows(AssertionError.class,
() -> assertToXContentEquivalent(BytesReference.bytes(builder), BytesReference.bytes(otherBuilder),
builder.contentType()));
assertThat(error.getMessage(), containsString("f2: expected [value2] but was [differentValue2]"));
assertThat(error.getMessage(), containsString("f2: expected String [value2] but was String [differentValue2]"));
}
{
XContentBuilder builder = JsonXContent.contentBuilder();
Expand Down Expand Up @@ -155,7 +155,7 @@ public void testAssertXContentEquivalentErrors() throws IOException {
AssertionError error = expectThrows(AssertionError.class,
() -> assertToXContentEquivalent(BytesReference.bytes(builder), BytesReference.bytes(otherBuilder),
builder.contentType()));
assertThat(error.getMessage(), containsString("2: expected [three] but was [four]"));
assertThat(error.getMessage(), containsString("2: expected String [three] but was String [four]"));
}
{
XContentBuilder builder = JsonXContent.contentBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public void test() throws IOException {
} catch (AssertionError ae) {
// Some tests assert on searches of wildcarded ML indices rather than on ML endpoints. For these we expect no hits.
if (ae.getMessage().contains("hits.total didn't match expected value")) {
assertThat(ae.getMessage(), containsString("but was [0]"));
assertThat(ae.getMessage(), containsString("but was Integer [0]"));
} else {
assertThat(ae.getMessage(),
either(containsString("action [cluster:monitor/xpack/ml")).or(containsString("action [cluster:admin/xpack/ml")));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,11 @@ public void testExplainWithWhere() throws IOException {
assertThat(readLine(), startsWith(" },"));
assertThat(readLine(), startsWith(" \"_source\" : {"));
assertThat(readLine(), startsWith(" \"includes\" : ["));
assertThat(readLine(), startsWith(" \"i\""));
assertThat(readLine(), startsWith(" \"test_field\""));
assertThat(readLine(), startsWith(" ],"));
assertThat(readLine(), startsWith(" \"excludes\" : [ ]"));
assertThat(readLine(), startsWith(" },"));
assertThat(readLine(), startsWith(" \"docvalue_fields\" : ["));
assertThat(readLine(), startsWith(" {"));
assertThat(readLine(), startsWith(" \"field\" : \"i\""));
assertThat(readLine(), startsWith(" }"));
assertThat(readLine(), startsWith(" ],"));
assertThat(readLine(), startsWith(" \"sort\" : ["));
assertThat(readLine(), startsWith(" {"));
assertThat(readLine(), startsWith(" \"_doc\" :"));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

package org.elasticsearch.xpack.sql.qa.single_node;

import org.elasticsearch.xpack.sql.qa.FieldExtractorTestCase;

public class FieldExtractorIT extends FieldExtractorTestCase {

}
Loading

0 comments on commit 74172ad

Please sign in to comment.