Skip to content

Commit

Permalink
fix various issues concerning custom schemas
Browse files Browse the repository at this point in the history
count(*) queries and delete queries swallowed the schema on tables with custom schemas
  • Loading branch information
msbt committed Mar 13, 2015
1 parent 55366dc commit 793d60a
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public static PartitionName toPartitionName(TableInfo tableInfo,
String.format("\"%s\" is no known partition column", entry.getKey().sqlFqn()));
}
}
return new PartitionName(tableInfo.ident().name(), Arrays.asList(values));
return new PartitionName(tableInfo.ident(), Arrays.asList(values));
}

public static String toPartitionIdent(TableInfo tableInfo,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public static String indexName(TableInfo tableInfo, Optional<List<BytesRef>> val
if (tableInfo.isPartitioned()){
return new PartitionName(tableInfo.ident(), values.get()).stringValue();
} else {
return tableInfo.ident().name();
return tableInfo.ident().esName();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
public class SysShardsTableInfo extends SysTableInfo {

public static final TableIdent IDENT = new TableIdent(SCHEMA, "shards");
private static final String[] CONCRETE_INDICES = new String[]{IDENT.name()};
private static final String[] CONCRETE_INDICES = new String[]{IDENT.esName()};

public static final Map<ColumnIdent, ReferenceInfo> INFOS = new LinkedHashMap<>(7);
private static final LinkedHashSet<ReferenceInfo> columns = new LinkedHashSet<>(7);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public Iterable<TableInfo> apply(SchemaInfo input) {
@Override
public boolean apply(TableInfo input) {
assert input != null;
return !PartitionName.isPartition(input.ident().name());
return !PartitionName.isPartition(input.ident().esName());
}
});
}
Expand Down
2 changes: 1 addition & 1 deletion sql/src/main/java/io/crate/planner/Planner.java
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ public static String[] indices(TableInfo tableInfo, WhereClause whereClause) {
indices = org.elasticsearch.common.Strings.EMPTY_ARRAY;
} else if (!tableInfo.isPartitioned()) {
// table name for non-partitioned tables
indices = new String[]{tableInfo.ident().name()};
indices = new String[]{tableInfo.ident().esName()};
} else if (whereClause.partitions().isEmpty()) {
if (whereClause.noMatch()) {
return new String[0];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
@CrateIntegrationTest.ClusterScope(scope = CrateIntegrationTest.Scope.GLOBAL)
public class CustomSchemaIntegrationTest extends SQLTransportIntegrationTest {


@Test
public void testInformationSchemaTablesReturnCorrectTablesIfCustomSchemaIsSimilarToTableName() throws Exception {
// regression test.. this caused foobar to be detected as a table in the foo schema and caused a NPE
Expand All @@ -43,4 +42,36 @@ public void testInformationSchemaTablesReturnCorrectTablesIfCustomSchemaIsSimila
"foo| bar\n" +
"doc| foobar\n"));
}

@Test
public void testDeleteFromCustomTable() throws Exception {
execute("create table custom.t (id int) with (number_of_replicas=0)");
ensureGreen();

This comment has been minimized.

Copy link
@mfussenegger

mfussenegger Mar 13, 2015

Member

no ensureGreen required

execute("insert into custom.t (id) values (?)", new Object[][]{{1}, {2}, {3}, {4}});
refresh();

execute("select count(*) from custom.t");
assertThat((Long)response.rows()[0][0], is(4L));

execute("delete from custom.t where id=1");
assertThat(response.rowCount(), is(-1L));

execute("select * from custom.t");
assertThat(response.rowCount(), is(3L));
}

This comment has been minimized.

Copy link
@mfussenegger

mfussenegger Mar 13, 2015

Member

can you also do a "delete by query" test ?


@Test
public void testGetCustomSchema() throws Exception {
execute("create table custom.t (id int) with (number_of_replicas=0)");
ensureGreen();
execute("insert into custom.t (id) values (?)", new Object[][]{{1}, {2}, {3}, {4}});
refresh();

execute("select id from custom.t where id=1");
assertThat(TestingHelpers.printedTable(response.rows()), is("1\n"));

execute("select id from custom.t where id in (2,4) order by id");
assertThat(TestingHelpers.printedTable(response.rows()), is("2\n4\n"));

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,6 @@ public void testSelectToGetRequestByPlanner() throws Exception {
assertThat(response.duration(), greaterThanOrEqualTo(0L));
}


@Test
public void testDeleteToDeleteRequestByPlanner() throws Exception {
this.setup.createTestTableWithPrimaryKey();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ public boolean isAlias() {

@Override
public String[] concreteIndices() {
return new String[]{ident.name()};
return new String[]{ident.esName()};
}

@Override
Expand Down
19 changes: 19 additions & 0 deletions sql/src/test/java/io/crate/planner/PlannerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1640,4 +1640,23 @@ public void testGroupByOnClusteredByColumnPartitionedOnePartition() throws Excep
Plan plan = plan("select count(*), city from clustered_parted where date=1395874800000 or date=1395961200000 group by city");
assertThat(plan, instanceOf(DistributedGroupBy.class));
}

@Test
public void testIndices() throws Exception {

This comment has been minimized.

Copy link
@mfussenegger

mfussenegger Mar 13, 2015

Member

testIndices isn't very descriptive :(

This comment has been minimized.

Copy link
@mfelsche

mfelsche Mar 13, 2015

Contributor

its the name of the method to test

TableIdent custom = new TableIdent("custom", "table");
String[] indices = Planner.indices(TestingTableInfo.builder(custom, RowGranularity.DOC, shardRouting).add("id", DataTypes.INTEGER, null).build(), WhereClause.MATCH_ALL);
assertThat(indices, arrayContainingInAnyOrder("custom.table"));

indices = Planner.indices(TestingTableInfo.builder(new TableIdent(null, "table"), RowGranularity.DOC, shardRouting).add("id", DataTypes.INTEGER, null).build(), WhereClause.MATCH_ALL);
assertThat(indices, arrayContainingInAnyOrder("table"));

indices = Planner.indices(TestingTableInfo.builder(custom, RowGranularity.DOC, shardRouting)
.add("id", DataTypes.INTEGER, null)
.add("date", DataTypes.TIMESTAMP, null, true)
.addPartitions(new PartitionName(custom, Arrays.asList(new BytesRef("0"))).stringValue())
.addPartitions(new PartitionName(custom, Arrays.asList(new BytesRef("12345"))).stringValue())
.build(), WhereClause.MATCH_ALL);
assertThat(indices, arrayContainingInAnyOrder("custom..partitioned.table.04130", "custom..partitioned.table.04332chj6gqg"));

}
}

0 comments on commit 793d60a

Please sign in to comment.