Browse Source

Shrink slow log for inner_hits (#84143)

Relates to #76515
Nik Everett 3 years ago
parent
commit
f3d622dd36

+ 1 - 13
modules/parent-join/src/test/java/org/elasticsearch/join/query/HasChildQueryBuilderTests.java

@@ -229,13 +229,7 @@ public class HasChildQueryBuilderTests extends AbstractQueryTestCase<HasChildQue
                 "_name" : "WNzYMJKRwePuRBh",
                 "inner_hits" : {
                   "name" : "inner_hits_name",
-                  "ignore_unmapped" : false,
-                  "from" : 0,
                   "size" : 100,
-                  "version" : false,
-                  "seq_no_primary_term" : false,
-                  "explain" : false,
-                  "track_scores" : false,
                   "sort" : [ {
                     "mapped_string" : {
                       "order" : "asc"
@@ -250,7 +244,7 @@ public class HasChildQueryBuilderTests extends AbstractQueryTestCase<HasChildQue
              * Ignoring unmapped is the default and we don't dump it and can't
              * change it if we're going to use inner_hits.
              */
-            query.replaceFirst("\"ignore_unmapped\" : false,", ""),
+            query.replaceAll("\"ignore_unmapped\" : false,", ""),
             queryBuilder
         );
         assertEquals(query, queryBuilder.maxChildren(), 1217235442);
@@ -317,13 +311,7 @@ public class HasChildQueryBuilderTests extends AbstractQueryTestCase<HasChildQue
                 "type" : "child",
                 "inner_hits" : {
                   "name" : "inner_hits_name",
-                  "ignore_unmapped" : false,
-                  "from" : 0,
                   "size" : 100,
-                  "version" : false,
-                  "seq_no_primary_term" : false,
-                  "explain" : false,
-                  "track_scores" : false,
                   "sort" : [ {
                     "mapped_string" : {
                       "order" : "asc"

+ 36 - 21
server/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java

@@ -48,6 +48,14 @@ public final class InnerHitBuilder implements Writeable, ToXContentObject {
     public static final ParseField COLLAPSE_FIELD = new ParseField("collapse");
     public static final ParseField FIELD_FIELD = new ParseField("field");
 
+    private static final boolean DEFAULT_IGNORE_UNAMPPED = false;
+    private static final int DEFAULT_FROM = 0;
+    private static final int DEFAULT_SIZE = 3;
+    private static final boolean DEFAULT_VERSION = false;
+    private static final boolean DEFAULT_SEQ_NO_AND_PRIMARY_TERM = false;
+    private static final boolean DEFAULT_EXPLAIN = false;
+    private static final boolean DEFAULT_TRACK_SCORES = false;
+
     private static final ObjectParser<InnerHitBuilder, Void> PARSER = new ObjectParser<>("inner_hits", InnerHitBuilder::new);
 
     static {
@@ -122,17 +130,16 @@ public final class InnerHitBuilder implements Writeable, ToXContentObject {
         }, COLLAPSE_FIELD, ObjectParser.ValueType.OBJECT);
     }
     private String name;
-    private boolean ignoreUnmapped;
+    private boolean ignoreUnmapped = DEFAULT_IGNORE_UNAMPPED;
 
-    private int from;
-    private int size = 3;
-    private boolean explain;
-    private boolean version;
-    private boolean seqNoAndPrimaryTerm;
-    private boolean trackScores;
+    private int from = DEFAULT_FROM;
+    private int size = DEFAULT_SIZE;
+    private boolean explain = DEFAULT_EXPLAIN;
+    private boolean version = DEFAULT_VERSION;
+    private boolean seqNoAndPrimaryTerm = DEFAULT_SEQ_NO_AND_PRIMARY_TERM;
+    private boolean trackScores = DEFAULT_TRACK_SCORES;
 
     private StoredFieldsContext storedFieldsContext;
-    private QueryBuilder query = DEFAULT_INNER_HIT_QUERY;
     private List<SortBuilder<?>> sorts;
     private List<FieldAndFormat> docValueFields;
     private Set<ScriptField> scriptFields;
@@ -463,10 +470,6 @@ public final class InnerHitBuilder implements Writeable, ToXContentObject {
         return this;
     }
 
-    QueryBuilder getQuery() {
-        return query;
-    }
-
     public InnerHitBuilder setInnerCollapse(CollapseBuilder innerCollapseBuilder) {
         this.innerCollapseBuilder = innerCollapseBuilder;
         return this;
@@ -482,13 +485,27 @@ public final class InnerHitBuilder implements Writeable, ToXContentObject {
         if (name != null) {
             builder.field(NAME_FIELD.getPreferredName(), name);
         }
-        builder.field(IGNORE_UNMAPPED.getPreferredName(), ignoreUnmapped);
-        builder.field(SearchSourceBuilder.FROM_FIELD.getPreferredName(), from);
-        builder.field(SearchSourceBuilder.SIZE_FIELD.getPreferredName(), size);
-        builder.field(SearchSourceBuilder.VERSION_FIELD.getPreferredName(), version);
-        builder.field(SearchSourceBuilder.SEQ_NO_PRIMARY_TERM_FIELD.getPreferredName(), seqNoAndPrimaryTerm);
-        builder.field(SearchSourceBuilder.EXPLAIN_FIELD.getPreferredName(), explain);
-        builder.field(SearchSourceBuilder.TRACK_SCORES_FIELD.getPreferredName(), trackScores);
+        if (ignoreUnmapped != DEFAULT_IGNORE_UNAMPPED) {
+            builder.field(IGNORE_UNMAPPED.getPreferredName(), ignoreUnmapped);
+        }
+        if (from != DEFAULT_FROM) {
+            builder.field(SearchSourceBuilder.FROM_FIELD.getPreferredName(), from);
+        }
+        if (size != DEFAULT_SIZE) {
+            builder.field(SearchSourceBuilder.SIZE_FIELD.getPreferredName(), size);
+        }
+        if (version != DEFAULT_VERSION) {
+            builder.field(SearchSourceBuilder.VERSION_FIELD.getPreferredName(), version);
+        }
+        if (seqNoAndPrimaryTerm != DEFAULT_SEQ_NO_AND_PRIMARY_TERM) {
+            builder.field(SearchSourceBuilder.SEQ_NO_PRIMARY_TERM_FIELD.getPreferredName(), seqNoAndPrimaryTerm);
+        }
+        if (explain != DEFAULT_EXPLAIN) {
+            builder.field(SearchSourceBuilder.EXPLAIN_FIELD.getPreferredName(), explain);
+        }
+        if (trackScores != DEFAULT_TRACK_SCORES) {
+            builder.field(SearchSourceBuilder.TRACK_SCORES_FIELD.getPreferredName(), trackScores);
+        }
         if (fetchSourceContext != null) {
             builder.field(SearchSourceBuilder._SOURCE_FIELD.getPreferredName(), fetchSourceContext, params);
         }
@@ -547,7 +564,6 @@ public final class InnerHitBuilder implements Writeable, ToXContentObject {
             && trackScores == that.trackScores
             && Objects.equals(name, that.name)
             && Objects.equals(storedFieldsContext, that.storedFieldsContext)
-            && Objects.equals(query, that.query)
             && Objects.equals(sorts, that.sorts)
             && Objects.equals(docValueFields, that.docValueFields)
             && Objects.equals(scriptFields, that.scriptFields)
@@ -569,7 +585,6 @@ public final class InnerHitBuilder implements Writeable, ToXContentObject {
             seqNoAndPrimaryTerm,
             trackScores,
             storedFieldsContext,
-            query,
             sorts,
             docValueFields,
             scriptFields,

+ 116 - 0
server/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java

@@ -29,6 +29,7 @@ import org.elasticsearch.xcontent.XContentBuilder;
 import org.elasticsearch.xcontent.XContentFactory;
 import org.elasticsearch.xcontent.XContentParser;
 import org.elasticsearch.xcontent.XContentType;
+import org.elasticsearch.xcontent.json.JsonXContent;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 
@@ -46,6 +47,7 @@ import static java.util.Collections.emptyList;
 import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.nullValue;
 import static org.hamcrest.Matchers.sameInstance;
 
 public class InnerHitBuilderTests extends ESTestCase {
@@ -328,4 +330,118 @@ public class InnerHitBuilderTests extends ESTestCase {
             innerHit.getFetchFields()
         );
     }
+
+    public void testParseDefaults() throws IOException {
+        InnerHitBuilder b = parse("{}");
+        assertThat(b.getName(), nullValue());
+        assertThat(b.getFrom(), equalTo(0));
+        assertThat(b.getSize(), equalTo(3));
+        assertThat(b.isVersion(), equalTo(false));
+        assertThat(b.isSeqNoAndPrimaryTerm(), equalTo(false));
+        assertThat(b.isExplain(), equalTo(false));
+        assertThat(b.isTrackScores(), equalTo(false));
+        assertThat(b.getStoredFieldsContext(), equalTo(null));
+        assertThat(b.getSorts(), equalTo(null));
+        assertThat(b.getDocValueFields(), equalTo(null));
+        assertThat(b.getScriptFields(), equalTo(null));
+        assertThat(b.getHighlightBuilder(), equalTo(null));
+        assertThat(b.getFetchSourceContext(), equalTo(null));
+        assertThat(b.getFetchFields(), equalTo(null));
+        assertThat(b.getInnerCollapseBuilder(), equalTo(null));
+        assertThat(Strings.toString(b), equalTo("{}"));
+    }
+
+    public void testParseDefaultsRemoved() throws IOException {
+        String json = """
+            {
+              "name" : "inner_hits_name",
+              "ignore_unmapped" : false,
+              "from" : 0,
+              "size" : 3,
+              "version" : false,
+              "seq_no_primary_term" : false,
+              "explain" : false,
+              "track_scores" : false
+            }""";
+        assertThat(Strings.toString(parse(json), true, true), equalTo("""
+            {
+              "name" : "inner_hits_name"
+            }"""));
+    }
+
+    public void testParseStoredFields() throws IOException {
+        InnerHitBuilder b = parse("""
+            {
+              "stored_fields" : ["foo"]
+            }""");
+        assertThat(b.getStoredFieldsContext().fieldNames(), equalTo(List.of("foo")));
+        assertThat(Strings.toString(b, true, true), equalTo("""
+            {
+              "stored_fields" : "foo"
+            }"""));
+
+        b = parse("""
+            {
+              "stored_fields" : ["foo", "bar"]
+            }""");
+        assertThat(b.getStoredFieldsContext().fieldNames(), equalTo(List.of("foo", "bar")));
+        assertThat(Strings.toString(b, true, true), equalTo("""
+            {
+              "stored_fields" : [
+                "foo",
+                "bar"
+              ]
+            }"""));
+
+        b = parse("""
+            {
+              "stored_fields" : ["_none_"]
+            }""");
+        assertThat(b.getStoredFieldsContext().fieldNames(), equalTo(null));
+        assertThat(b.getStoredFieldsContext().fetchFields(), equalTo(false));
+        assertThat(Strings.toString(b, true, true), equalTo("""
+            {
+              "stored_fields" : "_none_"
+            }"""));
+    }
+
+    public void testParseSorts() throws IOException {
+        InnerHitBuilder b = parse("""
+            {
+              "sort" : ["foo"]
+            }""");
+        assertThat(b.getSorts(), equalTo(List.of(SortBuilders.fieldSort("foo"))));
+        assertThat(Strings.toString(b, true, true), equalTo("""
+            {
+              "sort" : [
+                {
+                  "foo" : {
+                    "order" : "asc"
+                  }
+                }
+              ]
+            }"""));
+
+        b = parse("""
+            {
+              "sort" : [{"foo": "desc"}]
+            }""");
+        assertThat(b.getSorts(), equalTo(List.of(SortBuilders.fieldSort("foo").order(SortOrder.DESC))));
+        assertThat(Strings.toString(b, true, true), equalTo("""
+            {
+              "sort" : [
+                {
+                  "foo" : {
+                    "order" : "desc"
+                  }
+                }
+              ]
+            }"""));
+    }
+
+    private InnerHitBuilder parse(String json) throws IOException {
+        try (XContentParser parser = createParser(JsonXContent.jsonXContent, json)) {
+            return InnerHitBuilder.fromXContent(parser);
+        }
+    }
 }