From e800e1ca679f7e8d420c78cabe25a147a7ce8601 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Wed, 4 Jul 2018 15:16:21 +0300 Subject: [PATCH 1/3] SQL: Fix incorrect message for aliases Fix the naming in the verification message thrown for aliases over multiple indices with different mappings. --- .../sql/analysis/index/IndexResolver.java | 2 +- .../analysis/index/IndexResolverTests.java | 62 +++++++++++++++++++ .../src/test/resources/mapping-numeric.json | 16 +++++ 3 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolverTests.java create mode 100644 x-pack/plugin/sql/src/test/resources/mapping-numeric.json diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolver.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolver.java index 1800c170b7cff..10586c991b1ac 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolver.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolver.java @@ -253,7 +253,7 @@ static IndexResolution merge(List resolutions, String indexWild // need the same mapping across all resolutions if (!merged.get().mapping().equals(resolution.get().mapping())) { return IndexResolution.invalid( - "[" + indexWildcard + "] points to indices [" + resolution.get().name() + "] " + "[" + indexWildcard + "] points to indices [" + merged.get().name() + "] " + "and [" + resolution.get().name() + "] which have different mappings. " + "When using multiple indices, the mappings must be identical."); } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolverTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolverTests.java new file mode 100644 index 0000000000000..e43e31b7bb0b9 --- /dev/null +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolverTests.java @@ -0,0 +1,62 @@ +/* + * 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.analysis.index; + +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.sql.type.EsField; +import org.elasticsearch.xpack.sql.type.TypesTests; +import org.junit.Test; + +import java.util.Arrays; +import java.util.Map; + +public class IndexResolverTests extends ESTestCase { + + @Test + public void testMergeSameMapping() throws Exception { + Map oneMapping = TypesTests.loadMapping("mapping-basic.json", true); + Map sameMapping = TypesTests.loadMapping("mapping-basic.json", true); + assertNotSame(oneMapping, sameMapping); + assertEquals(oneMapping, sameMapping); + + String wildcard = "*"; + IndexResolution resolution = IndexResolver.merge( + Arrays.asList(IndexResolution.valid(new EsIndex("a", oneMapping)), IndexResolution.valid(new EsIndex("b", sameMapping))), + wildcard); + + assertTrue(resolution.isValid()); + + EsIndex esIndex = resolution.get(); + + assertEquals(wildcard, esIndex.name()); + assertEquals(sameMapping, esIndex.mapping()); + } + + @Test + public void testMergeDifferentMapping() throws Exception { + Map oneMapping = TypesTests.loadMapping("mapping-basic.json", true); + Map sameMapping = TypesTests.loadMapping("mapping-basic.json", true); + Map differentMapping = TypesTests.loadMapping("mapping-numeric.json", true); + + assertNotSame(oneMapping, sameMapping); + assertEquals(oneMapping, sameMapping); + assertNotEquals(oneMapping, differentMapping); + + String wildcard = "*"; + IndexResolution resolution = IndexResolver.merge( + Arrays.asList(IndexResolution.valid(new EsIndex("a", oneMapping)), + IndexResolution.valid(new EsIndex("b", sameMapping)), + IndexResolution.valid(new EsIndex("diff", differentMapping))), + wildcard); + + assertFalse(resolution.isValid()); + + MappingException ex = expectThrows(MappingException.class, () -> resolution.get()); + assertEquals( + "[*] points to indices [a] and [diff] which have different mappings. When using multiple indices, the mappings must be identical.", + ex.getMessage()); + } +} diff --git a/x-pack/plugin/sql/src/test/resources/mapping-numeric.json b/x-pack/plugin/sql/src/test/resources/mapping-numeric.json new file mode 100644 index 0000000000000..a95ecfb3aa706 --- /dev/null +++ b/x-pack/plugin/sql/src/test/resources/mapping-numeric.json @@ -0,0 +1,16 @@ +{ + "properties" : { + "byte" : { + "type" : "byte" + }, + "short" : { + "type" : "short" + }, + "integer" : { + "type" : "integer" + }, + "long" : { + "type" : "long" + } + } +} From 551e4512e87cab1dcf6cfecf0df9afce2a8242d9 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Thu, 5 Jul 2018 12:05:46 +0300 Subject: [PATCH 2/3] Fix long line --- .../xpack/sql/analysis/index/IndexResolverTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolverTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolverTests.java index e43e31b7bb0b9..0c9564587af6a 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolverTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolverTests.java @@ -56,7 +56,8 @@ public void testMergeDifferentMapping() throws Exception { MappingException ex = expectThrows(MappingException.class, () -> resolution.get()); assertEquals( - "[*] points to indices [a] and [diff] which have different mappings. When using multiple indices, the mappings must be identical.", + "[*] points to indices [a] and [diff] which have different mappings. " + + "When using multiple indices, the mappings must be identical.", ex.getMessage()); } } From 0a87747d647b5a5fc8db79c47175cf0298f92a08 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Thu, 5 Jul 2018 14:16:15 +0300 Subject: [PATCH 3/3] Fix forbidden api violation --- .../xpack/sql/analysis/index/IndexResolverTests.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolverTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolverTests.java index 0c9564587af6a..639356b2997f9 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolverTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolverTests.java @@ -8,14 +8,12 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.sql.type.EsField; import org.elasticsearch.xpack.sql.type.TypesTests; -import org.junit.Test; import java.util.Arrays; import java.util.Map; public class IndexResolverTests extends ESTestCase { - @Test public void testMergeSameMapping() throws Exception { Map oneMapping = TypesTests.loadMapping("mapping-basic.json", true); Map sameMapping = TypesTests.loadMapping("mapping-basic.json", true); @@ -35,7 +33,6 @@ public void testMergeSameMapping() throws Exception { assertEquals(sameMapping, esIndex.mapping()); } - @Test public void testMergeDifferentMapping() throws Exception { Map oneMapping = TypesTests.loadMapping("mapping-basic.json", true); Map sameMapping = TypesTests.loadMapping("mapping-basic.json", true);