Skip to content

Commit

Permalink
SQL: Fix querying of indices without columns (#74312) (#74465)
Browse files Browse the repository at this point in the history
Fixes #53001

Adds support for querying indices without columns. Previously, such queries failed.

(cherry picked from commit 6f93303)
  • Loading branch information
Lukas Wegmann committed Jun 23, 2021
1 parent 0637218 commit ae96371
Show file tree
Hide file tree
Showing 15 changed files with 124 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,6 @@ public void testSelectColumnFromMissingIndex() throws SQLException {
}
}

public void testSelectFromEmptyIndex() throws IOException, SQLException {
// 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());
assertEquals("Found 1 problem\nline 1:8: Cannot determine columns for [*]", e.getMessage());
}
}

public void testSelectColumnFromEmptyIndex() throws IOException, SQLException {
Request request = new Request("PUT", "/test");
request.setJsonEntity("{}");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ public interface ErrorsTestCase {

void testSelectColumnFromMissingIndex() throws Exception;

void testSelectFromEmptyIndex() throws Exception;

void testSelectColumnFromEmptyIndex() throws Exception;

void testSelectMissingField() throws Exception;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,6 @@ public void testSelectColumnFromMissingIndex() throws Exception {
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:8: Cannot determine columns for [*]" + END, readLine());
}

@Override
public void testSelectColumnFromEmptyIndex() throws Exception {
Request request = new Request("PUT", "/test");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,15 @@ protected static void loadEmpDatasetIntoEs(RestClient client) throws Exception {
// frozen index
loadEmpDatasetIntoEs(client, "frozen_emp", "employees");
freeze(client, "frozen_emp");
loadNoColsDatasetIntoEs(client, "empty_mapping");
}

private static void loadNoColsDatasetIntoEs(RestClient client, String index) throws Exception {
createEmptyIndex(client, index);
Request request = new Request("POST", "/" + index + "/_bulk");
request.addParameter("refresh", "true");
request.setJsonEntity("{\"index\":{}\n{}\n" + "{\"index\":{}\n{}\n");
client.performRequest(request);
}

public static void loadDocsDatasetIntoEs(RestClient client) throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ public abstract class SqlSpecTestCase extends SpecBaseIntegrationTestCase {
private String query;

@ClassRule
public static LocalH2 H2 = new LocalH2(c -> c.createStatement().execute("RUNSCRIPT FROM 'classpath:/setup_test_emp.sql'"));
public static LocalH2 H2 = new LocalH2((c) -> {
c.createStatement().execute("RUNSCRIPT FROM 'classpath:/setup_test_emp.sql'");
c.createStatement().execute("RUNSCRIPT FROM 'classpath:/setup_empty_mapping.sql'");
});

@ParametersFactory(argumentFormatting = PARAM_FORMATTING)
public static List<Object[]> readScriptSpec() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,16 +344,6 @@ public void testSelectColumnFromMissingIndex() throws Exception {
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"), containsString("1:8: Cannot determine columns for [*]"));
}

@Override
public void testSelectColumnFromEmptyIndex() throws Exception {
Request request = new Request("PUT", "/test");
Expand Down
25 changes: 21 additions & 4 deletions x-pack/plugin/sql/qa/server/src/main/resources/command.csv-spec
Original file line number Diff line number Diff line change
Expand Up @@ -223,13 +223,14 @@ TODAY |SCALAR
showTables
SHOW TABLES;

name | type | kind
name | type | kind
empty_mapping |TABLE |INDEX
logs |TABLE |INDEX
logs_nanos |TABLE |INDEX
test_alias |VIEW |ALIAS
test_alias_emp |VIEW |ALIAS
test_emp |TABLE |INDEX
test_emp_copy |TABLE |INDEX
test_alias_emp |VIEW |ALIAS
test_emp |TABLE |INDEX
test_emp_copy |TABLE |INDEX
;

showTablesSimpleLike
Expand Down Expand Up @@ -382,3 +383,19 @@ last_name |VARCHAR |text
last_name.keyword |VARCHAR |keyword
salary |INTEGER |integer
;


describeNoCols
DESCRIBE "empty_mapping";

column:s | type:s | mapping:s
----------------------+---------------+---------------
;


showColumnsInNoCols
SHOW COLUMNS IN "empty_mapping";

column:s | type:s | mapping:s
----------------------+---------------+---------------
;
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@

// the following queries all return no rows in H2

selectConstAggregationWithGroupBy
SELECT 1 a, COUNT(*) b, MAX(1) c FROM empty_mapping GROUP BY a;

a:i | b:l | c:i
---------------+---------------+---------------
1 |2 |1
;

subselectWithConst
SELECT 1, * FROM (SELECT * FROM empty_mapping) s;

1:i
---------------
1
1
;

subselectWithInnerConst
SELECT * FROM (SELECT 1, * FROM empty_mapping) s;

1:i
---------------
1
1
;
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
selectStar
SELECT * FROM empty_mapping;
selectStarWithFilter
SELECT * FROM empty_mapping WHERE 2 > 1;
selectStarWithFilterAndLimit
SELECT * FROM empty_mapping WHERE 1 > 2 LIMIT 10;

selectCount
SELECT COUNT(*) FROM empty_mapping;
// awaits fix: https://github.com/elastic/elasticsearch/issues/74311
// selectCountWithWhere
// SELECT COUNT(*) FROM empty_mapping WHERE 1 + 1 = 3;

selectConst
SELECT 1, 2, 3 FROM empty_mapping;
selectConstAggregation
SELECT MAX(1), SUM(2) FROM empty_mapping;

// fails in H2 with a syntax error but cannot be tested in CSV spec because datasets without columns cannot be parsed
// awaits fix: https://github.com/elastic/elasticsearch/issues/39895 (latest H2 version can run the query)
// subselect
// SELECT * FROM (SELECT * FROM empty_mapping) s;
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
DROP TABLE IF EXISTS "empty_mapping";
CREATE TABLE "empty_mapping" ();

INSERT INTO "empty_mapping" DEFAULT VALUES;
INSERT INTO "empty_mapping" DEFAULT VALUES;
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
DROP TABLE IF EXISTS "test_emp";
CREATE TABLE "test_emp" (
"birth_date" TIMESTAMP WITH TIME ZONE,
"emp_no" INT,
"emp_no" INT,
"first_name" VARCHAR(50),
"gender" VARCHAR(1),
"hire_date" TIMESTAMP WITH TIME ZONE,
Expand All @@ -10,4 +10,5 @@ CREATE TABLE "test_emp" (
"name" VARCHAR(50),
"salary" INT
)
AS SELECT birth_date, emp_no, first_name, gender, hire_date, languages, last_name, CONCAT(first_name, ' ', last_name) AS name, salary FROM CSVREAD('classpath:/employees.csv');
AS SELECT birth_date, emp_no, first_name, gender, hire_date, languages, last_name, CONCAT(first_name, ' ', last_name) AS name, salary FROM CSVREAD('classpath:/employees.csv');

Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@
showTables
SHOW TABLES INCLUDE FROZEN;

name | type | kind
name | type | kind
empty_mapping |TABLE |INDEX
frozen_emp |TABLE |FROZEN INDEX
logs |TABLE |INDEX
logs_nanos |TABLE |INDEX
test_alias |VIEW |ALIAS
test_alias_emp |VIEW |ALIAS
test_emp |TABLE |INDEX
test_emp_copy |TABLE |INDEX
test_alias_emp |VIEW |ALIAS
test_emp |TABLE |INDEX
test_emp_copy |TABLE |INDEX
;

columnFromFrozen
Expand All @@ -31,18 +32,18 @@ F
percentileFrozen
SELECT gender, PERCENTILE(emp_no, 92.45) p1 FROM FROZEN frozen_emp GROUP BY gender;

gender:s | p1:d
null |10018.745
gender:s | p1:d
null |10018.745
F |10098.0085
M |10091.393
M |10091.393
;

countFromFrozen
SELECT gender, COUNT(*) AS c FROM FROZEN frozen_emp GROUP BY gender;

gender:s | c:l
null |10
F |33
null |10
F |33
M |57
;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -417,12 +417,9 @@ private List<NamedExpression> expandProjections(List<? extends NamedExpression>
for (NamedExpression ne : projections) {
if (ne instanceof UnresolvedStar) {
List<NamedExpression> expanded = expandStar((UnresolvedStar) ne, output);

// the field exists, but cannot be expanded (no sub-fields)
if (expanded.isEmpty()) {
result.add(ne);
} else {
result.addAll(expanded);
}
result.addAll(expanded);
} else if (ne instanceof UnresolvedAlias) {
UnresolvedAlias ua = (UnresolvedAlias) ne;
if (ua.child() instanceof UnresolvedStar) {
Expand All @@ -442,7 +439,7 @@ private List<NamedExpression> expandStar(UnresolvedStar us, List<Attribute> outp
// a qualifier is specified - since this is a star, it should be a CompoundDataType
if (us.qualifier() != null) {
// resolve the so-called qualifier first
// since this is an unresolved start we don't know whether it's a path or an actual qualifier
// since this is an unresolved star we don't know whether it's a path or an actual qualifier
Attribute q = resolveAgainstList(us.qualifier(), output, true);

// the wildcard couldn't be expanded because the field doesn't exist at all
Expand All @@ -455,6 +452,10 @@ private List<NamedExpression> expandStar(UnresolvedStar us, List<Attribute> outp
else if (q.resolved() == false) {
return singletonList(q);
}
// qualifier resolves to a non-struct field and cannot be expanded
else if (DataTypes.isPrimitive(q.dataType())) {
return singletonList(us);
}

// now use the resolved 'qualifier' to match
for (Attribute attr : output) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,9 @@ public List<Tuple<Integer, Comparator>> sortingColumns() {
*/
public BitSet columnMask(List<Attribute> columns) {
BitSet mask = new BitSet(fields.size());
aliasName(columns.get(0));
if (columns.size() > 0) {
aliasName(columns.get(0));
}

for (Attribute column : columns) {
Expression expression = aliases.resolve(column, column);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.elasticsearch.xpack.sql.parser.SqlParser;
import org.elasticsearch.xpack.sql.stats.Metrics;

import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -301,4 +302,15 @@ public void testFunctionWithExpressionOverNonExistingFieldAsArgumentAndSameAlias
assertEquals("Found 1 problem\nline 1:22: Unknown column [missing]", ex.getMessage());
}

public void testExpandStarOnIndexWithoutColumns() {
EsIndex test = new EsIndex("test", Collections.emptyMap());
getIndexResult = IndexResolution.valid(test);
analyzer = new Analyzer(SqlTestUtils.TEST_CFG, functionRegistry, getIndexResult, verifier);

LogicalPlan plan = plan("SELECT * FROM test");

assertThat(plan, instanceOf(Project.class));
assertTrue(((Project) plan).projections().isEmpty());
}

}

0 comments on commit ae96371

Please sign in to comment.