Skip to content

Commit

Permalink
Fix: COPY FROM PARTITION failed if the source contained a ``partition…
Browse files Browse the repository at this point in the history 
…ed by`` column:

- add partitioned_by columns to mapping (index=no)
- DocIndexMetaData: ignore columns if there is a
partitioned by equivalent
  • Loading branch information
Philipp Bogensberger committed Feb 6, 2015
1 parent 7ce5866 commit 27538d1
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 28 deletions.
2 changes: 2 additions & 0 deletions CHANGES.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ Changes for Crate
Unreleased
==========

- Fix: COPY FROM PARTITION failed if the source contained a ``partitioned by`` column

- Fix: Adding new columns to existing partitions could fail if the table was altered
before

Expand Down
20 changes: 7 additions & 13 deletions sql/src/main/java/io/crate/analyze/AnalyzedTableElements.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.common.collect.ImmutableMap;
import io.crate.exceptions.ColumnUnknownException;
import io.crate.metadata.ColumnIdent;
import io.crate.metadata.ReferenceInfo;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;

Expand Down Expand Up @@ -202,7 +203,7 @@ public Set<ColumnIdent> columnIdents() {
}

@Nullable
private AnalyzedColumnDefinition columnDefinitionByIdent(ColumnIdent ident, boolean removeIfFound) {
private AnalyzedColumnDefinition columnDefinitionByIdent(ColumnIdent ident) {
AnalyzedColumnDefinition result = null;
ColumnIdent root = ident.getRoot();
for (AnalyzedColumnDefinition column : columns) {
Expand All @@ -216,33 +217,25 @@ private AnalyzedColumnDefinition columnDefinitionByIdent(ColumnIdent ident, bool
}

if (result.ident().equals(ident)) {
if (removeIfFound) {
columns.remove(result);
}
return result;
}

return findInChildren(result, ident, removeIfFound);
return findInChildren(result, ident);
}

private AnalyzedColumnDefinition findInChildren(AnalyzedColumnDefinition column,
ColumnIdent ident,
boolean removeIfFound) {
ColumnIdent ident) {
AnalyzedColumnDefinition result = null;
for (AnalyzedColumnDefinition child : column.children()) {
if (child.ident().equals(ident)) {
result = child;
break;
}
AnalyzedColumnDefinition inChildren = findInChildren(child, ident, removeIfFound);
AnalyzedColumnDefinition inChildren = findInChildren(child, ident);
if (inChildren != null) {
return inChildren;
}
}

if (removeIfFound && result != null) {
column.children().remove(result);
}
return result;
}

Expand All @@ -257,7 +250,7 @@ public void changeToPartitionedByColumn(ColumnIdent partitionedByIdent, boolean
partitionedByIdent.sqlFqn()));
}

AnalyzedColumnDefinition columnDefinition = columnDefinitionByIdent(partitionedByIdent, true);
AnalyzedColumnDefinition columnDefinition = columnDefinitionByIdent(partitionedByIdent);
if (columnDefinition == null) {
if (skipIfNotFound) {
return;
Expand All @@ -280,6 +273,7 @@ public void changeToPartitionedByColumn(ColumnIdent partitionedByIdent, boolean
columnDefinition.ident().sqlFqn()));
}
columnIdents.remove(columnDefinition.ident());
columnDefinition.index(ReferenceInfo.IndexType.NO.toString());
partitionedByColumns.add(columnDefinition);
}

Expand Down
9 changes: 6 additions & 3 deletions sql/src/main/java/io/crate/metadata/doc/DocIndexMetaData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,13 @@ private void add(ColumnIdent column, DataType type, ReferenceInfo.IndexType inde
private void add(ColumnIdent column, DataType type, ColumnPolicy columnPolicy,
ReferenceInfo.IndexType indexType, boolean partitioned) {
ReferenceInfo info = newInfo(column, type, columnPolicy, indexType);
if (info.ident().isColumn()) {
columnsBuilder.add(info);
// don't add it if there is a partitioned equivalent of this column
if(partitioned || !(partitionedBy != null && partitionedBy.contains(column))){
if (info.ident().isColumn()) {
columnsBuilder.add(info);
}
referencesBuilder.put(info.ident().columnIdent(), info);
}
referencesBuilder.put(info.ident().columnIdent(), info);
if (partitioned) {
partitionedByColumnsBuilder.add(info);
}
Expand Down
2 changes: 1 addition & 1 deletion sql/src/main/java/io/crate/metadata/doc/DocTableInfoBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public DocTableInfoBuilder(DocSchemaInfo docSchemaInfo,
this.checkAliasSchema = checkAliasSchema;
}

public DocIndexMetaData docIndexMetaData() {
private DocIndexMetaData docIndexMetaData() {
DocIndexMetaData docIndexMetaData;
String templateName = PartitionName.templateName(ident.schema(), ident.name());
boolean createdFromTemplate = false;
Expand Down
29 changes: 18 additions & 11 deletions sql/src/test/java/io/crate/analyze/CreateAlterPartitionedTableAnalyzerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

package io.crate.analyze;

import com.google.common.base.Joiner;
import io.crate.metadata.PartitionName;
import io.crate.exceptions.ColumnUnknownException;
import io.crate.metadata.FulltextAnalyzerResolver;
Expand Down Expand Up @@ -102,8 +103,11 @@ public void testPartitionedBy() throws Exception {
assertThat(analysis.partitionedBy().size(), is(1));
assertThat(analysis.partitionedBy().get(0), contains("name", "string"));

// partitioned columns should not appear as regular columns
assertThat(analysis.mappingProperties(), not(hasKey("name")));
// partitioned columns must be not indexed in mapping
Map<String, Object> nameMapping = (Map<String, Object>)analysis.mappingProperties().get("name");
assertThat(Joiner.on(", ").withKeyValueSeparator(":").join(nameMapping), is(
"index:no, store:false, doc_values:false, type:string"));

Map<String, Object> metaMapping = (Map) analysis.mapping().get("_meta");
assertThat((Map<String, Object>) metaMapping.get("columns"), not(hasKey("name")));
List<List<String>> partitionedByMeta = (List<List<String>>)metaMapping.get("partitioned_by");
Expand All @@ -116,16 +120,14 @@ public void testPartitionedBy() throws Exception {
@Test
public void testPartitionedByMultipleColumns() throws Exception {
CreateTableAnalyzedStatement analysis = (CreateTableAnalyzedStatement) analyze("create table my_table (" +
" id integer," +
" no_index string index off," +
" name string," +
" date timestamp" +
") partitioned by (name, date)");
assertThat(analysis.partitionedBy().size(), is(2));
assertThat(analysis.mappingProperties(), allOf(
not(hasKey("name")),
not(hasKey("date"))
));
Map<String, Object> properties = analysis.mappingProperties();
assertThat(Joiner.on(", ").withKeyValueSeparator(":").join(properties), is(
"date:{index=no, store=false, doc_values=false, type=date}, " +
"name:{index=no, store=false, doc_values=false, type=string}"));
assertThat((Map<String, Object>) ((Map) analysis.mapping().get("_meta")).get("columns"),
allOf(
not(hasKey("name")),
Expand All @@ -146,8 +148,10 @@ public void testPartitionedByNestedColumns() throws Exception {
" date timestamp" +
") partitioned by (date, o['name'])");
assertThat(analysis.partitionedBy().size(), is(2));
assertThat(analysis.mappingProperties(), not(hasKey("name")));
assertThat((Map<String, Object>) ((Map) analysis.mappingProperties().get("o")).get("properties"), not(hasKey("name")));
Map<String, Object> oMapping = (Map<String, Object>)analysis.mappingProperties().get("o");
assertThat(Joiner.on(", ").withKeyValueSeparator(":").join(oMapping), is(
"dynamic:true, index:not_analyzed, store:false, properties:{"+
"name={index=no, store=false, doc_values=false, type=string}}, doc_values:false, type:object"));
assertThat((Map<String, Object>) ((Map) analysis.mapping().get("_meta")).get("columns"), not(hasKey("date")));

Map metaColumns = (Map) ((Map) analysis.mapping().get("_meta")).get("columns");
Expand Down Expand Up @@ -224,7 +228,10 @@ public void testPartitionedByPartOfPrimaryKey() throws Exception {
") partitioned by (id1)");
assertThat(analysis.partitionedBy().size(), is(1));
assertThat(analysis.partitionedBy().get(0), contains("id1", "integer"));
assertThat(analysis.mappingProperties(), not(hasKey("id1")));

Map<String, Object> oMapping = (Map<String, Object>)analysis.mappingProperties().get("id1");
assertThat(Joiner.on(", ").withKeyValueSeparator(":").join(oMapping), is(
"index:no, store:false, doc_values:false, type:integer"));
assertThat((Map<String, Object>) ((Map) analysis.mapping().get("_meta")).get("columns"),
not(hasKey("id1")));
}
Expand Down
14 changes: 14 additions & 0 deletions sql/src/test/java/io/crate/integrationtests/CopyIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.junit.rules.ExpectedException;
import org.junit.rules.TemporaryFolder;

import java.io.File;
import java.nio.charset.StandardCharsets;
import java.nio.file.DirectoryStream;
import java.nio.file.Files;
Expand Down Expand Up @@ -127,6 +128,19 @@ public void testCopyFromFilePattern() throws Exception {
assertEquals(3L, response.rowCount());
}

@Test
public void testCopyFromFileWithPartition() throws Exception {
execute("create table quotes (id int, " +
"quote string) partitioned by (id)");
ensureGreen();
String filePath = Joiner.on(File.separator).join(copyFilePath, "test_copy_from.json");
execute("copy quotes partition (id = 1) from ? with (shared=true)", new Object[]{filePath});
refresh();

execute("select * from quotes");
assertEquals(3L, response.rowCount());
}

@Test
public void testCopyToFile() throws Exception {
execute("create table singleshard (name string) clustered into 1 shards with (number_of_replicas = 0)");
Expand Down
67 changes: 67 additions & 0 deletions sql/src/test/java/io/crate/metadata/doc/DocIndexMetaDataTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,73 @@ public void testExtractPartitionedByColumns() throws Exception {
assertThat(md.partitionedByColumns().get(0).ident().columnIdent().fqn(), is("datum"));
}

@Test
public void testExtractPartitionedByWithPartitionedByInColumns() throws Exception {
XContentBuilder builder = XContentFactory.jsonBuilder()
.startObject()
.startObject("_meta")
.startArray("partitioned_by")
.startArray()
.value("datum").value("date")
.endArray()
.endArray()
.endObject()
.startObject("properties")
.startObject("id")
.field("type", "integer")
.field("index", "not_analyzed")
.endObject()
.startObject("datum")
.field("type", "date")
.field("index", "not_analyzed")
.endObject()
.endObject()
.endObject();
IndexMetaData metaData = getIndexMetaData("test1", builder);
DocIndexMetaData md = newMeta(metaData, "test1");

// partitioned by column is not added twice
assertEquals(2, md.columns().size());
assertEquals(8, md.references().size());
assertEquals(1, md.partitionedByColumns().size());
}

@Test
public void testExtractPartitionedByWithNestedPartitionedByInColumns() throws Exception {
XContentBuilder builder = XContentFactory.jsonBuilder()
.startObject()
.startObject("_meta")
.startArray("partitioned_by")
.startArray()
.value("nested.datum").value("date")
.endArray()
.endArray()
.endObject()
.startObject("properties")
.startObject("id")
.field("type", "integer")
.field("index", "not_analyzed")
.endObject()
.startObject("nested")
.field("type", "nested")
.startObject("properties")
.startObject("datum")
.field("type", "date")
.field("index", "not_analyzed")
.endObject()
.endObject()
.endObject()
.endObject()
.endObject();
IndexMetaData metaData = getIndexMetaData("test1", builder);
DocIndexMetaData md = newMeta(metaData, "test1");

// partitioned by column is not added twice
assertEquals(2, md.columns().size());
assertEquals(9, md.references().size());
assertEquals(1, md.partitionedByColumns().size());
}

private Map<String, Object> sortProperties(Map<String, Object> mappingSource) {
return sortProperties(mappingSource, false);
}
Expand Down

0 comments on commit 27538d1

Please sign in to comment.