Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SQL: use a proper error message for queries directed at empty mapping indices #52967

Merged
merged 2 commits into from
Mar 2, 2020
Merged

SQL: use a proper error message for queries directed at empty mapping indices #52967

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9f7b26
Issue a different error message in case an index doesn't have a mapping
astefan Feb 28, 2020
4ca6492
Merge branch 'master' of https://github.com/elastic/elasticsearch int…
astefan Feb 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/IndexResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ public void resolveAsMergedMapping(String indexWildcard, String javaRegex, boole
public static IndexResolution mergedMappings(DataTypeRegistry typeRegistry, String indexPattern, String[] indexNames,
Map<String, Map<String, FieldCapabilities>> fieldCaps) {

if (fieldCaps == null || fieldCaps.isEmpty()) {
if (indexNames.length == 0) {
return IndexResolution.notFound(indexPattern);
}

Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/ErrorsTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
public interface ErrorsTestCase {
void testSelectInvalidSql() throws Exception;
void testSelectFromMissingIndex() throws Exception;
void testSelectFromIndexWithoutTypes() throws Exception;
void testSelectColumnFromMissingIndex() throws Exception;
void testSelectFromEmptyIndex() throws Exception;
void testSelectColumnFromEmptyIndex() throws Exception;
void testSelectMissingField() throws Exception;
void testSelectMissingFunction() throws Exception;
void testSelectProjectScoreInAggContext() throws Exception;
Expand Down
21 changes: 18 additions & 3 deletions x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/cli/ErrorsTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,30 @@ public void testSelectFromMissingIndex() throws IOException {
}

@Override
public void testSelectFromIndexWithoutTypes() throws Exception {
public void testSelectColumnFromMissingIndex() throws Exception {
assertFoundOneProblem(command("SELECT abc FROM test"));
assertEquals("line 1:17: Unknown index [test]" + END, readLine());
}

@Override
public void testSelectFromEmptyIndex() throws Exception {
// Create an index without any types
Request request = new Request("PUT", "/test");
request.setJsonEntity("{}");
client().performRequest(request);

assertFoundOneProblem(command("SELECT * FROM test"));
//assertEquals("line 1:15: [test] doesn't have any types so it is incompatible with sql" + END, readLine());
assertEquals("line 1:15: Unknown index [test]" + END, readLine());
assertEquals("line 1:8: Cannot determine columns for [*]" + END, readLine());
Copy link
Member

Choose a reason for hiding this comment

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

I think this is incorrect - if we know the index is empty, we should return no columns since we can determine them and know there are none.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created #53001

}

@Override
public void testSelectColumnFromEmptyIndex() throws Exception {
Request request = new Request("PUT", "/test");
request.setJsonEntity("{}");
client().performRequest(request);

assertFoundOneProblem(command("SELECT abc FROM test"));
assertEquals("line 1:8: Unknown column [abc]" + END, readLine());
}

@Override
Expand Down
26 changes: 22 additions & 4 deletions x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ErrorsTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,35 @@ public void testSelectFromMissingIndex() throws SQLException {
}

@Override
public void testSelectFromIndexWithoutTypes() throws Exception {
public void testSelectColumnFromMissingIndex() throws Exception {
try (Connection c = esJdbc()) {
SQLException e = expectThrows(SQLException.class, () -> c.prepareStatement("SELECT abc FROM test").executeQuery());
assertEquals("Found 1 problem\nline 1:17: Unknown index [test]", e.getMessage());
}
}

@Override
public void testSelectFromEmptyIndex() throws Exception {
// Create an index without any types
Request request = new Request("PUT", "/test");
request.setJsonEntity("{}");
client().performRequest(request);

try (Connection c = esJdbc()) {
SQLException e = expectThrows(SQLException.class, () -> c.prepareStatement("SELECT * FROM test").executeQuery());
// see https://github.com/elastic/elasticsearch/issues/34719
//assertEquals("Found 1 problem\nline 1:15: [test] doesn't have any types so it is incompatible with sql", e.getMessage());
assertEquals("Found 1 problem\nline 1:15: Unknown index [test]", e.getMessage());
assertEquals("Found 1 problem\nline 1:8: Cannot determine columns for [*]", e.getMessage());
}
}

@Override
public void testSelectColumnFromEmptyIndex() throws Exception {
Request request = new Request("PUT", "/test");
request.setJsonEntity("{}");
client().performRequest(request);

try (Connection c = esJdbc()) {
SQLException e = expectThrows(SQLException.class, () -> c.prepareStatement("SELECT abc FROM test").executeQuery());
assertEquals("Found 1 problem\nline 1:8: Unknown column [abc]", e.getMessage());
}
}

Expand Down
22 changes: 17 additions & 5 deletions x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,16 +317,28 @@ public void testSelectFromMissingIndex() {
}

@Override
public void testSelectFromIndexWithoutTypes() throws Exception {
public void testSelectColumnFromMissingIndex() throws Exception {
String mode = randomFrom("jdbc", "plain");
expectBadRequest(() -> runSql(mode, "SELECT abc FROM missing"), containsString("1:17: Unknown index [missing]"));
}

@Override
public void testSelectFromEmptyIndex() throws Exception {
// Create an index without any types
Request request = new Request("PUT", "/test");
request.setJsonEntity("{}");
client().performRequest(request);
String mode = randomFrom("jdbc", "plain");
expectBadRequest(() -> runSql(mode, "SELECT * FROM test"),
// see https://github.com/elastic/elasticsearch/issues/34719
//containsString("1:15: [test] doesn't have any types so it is incompatible with sql"));
containsString("1:15: Unknown index [test]"));
expectBadRequest(() -> runSql(mode, "SELECT * FROM test"), containsString("1:8: Cannot determine columns for [*]"));
}

@Override
public void testSelectColumnFromEmptyIndex() throws Exception {
Request request = new Request("PUT", "/test");
request.setJsonEntity("{}");
client().performRequest(request);
String mode = randomFrom("jdbc", "plain");
expectBadRequest(() -> runSql(mode, "SELECT abc FROM test"), containsString("1:8: Unknown column [abc]"));
}

@Override
Expand Down