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.

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

[fix](build index)Fix build index failed on renamed column #42882

Merged
merged 1 commit into from
Nov 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 9 additions & 1 deletion be/src/olap/tablet_schema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,15 @@ void TabletIndex::init_from_thrift(const TOlapTableIndex& index,
if (column_idx >= 0) {
col_unique_ids[i] = tablet_schema.column(column_idx).unique_id();
} else {
col_unique_ids[i] = -1;
// if column unique id not found by column name, find by column unique id
// column unique id can not bigger than tablet schema column size, if bigger than column size means
// this column is a new column added by light schema change
if (index.__isset.column_unique_ids &&
index.column_unique_ids[i] < tablet_schema.num_columns()) {
col_unique_ids[i] = index.column_unique_ids[i];
} else {
col_unique_ids[i] = -1;
}
}
}
_col_unique_ids = std::move(col_unique_ids);
Expand Down
12 changes: 8 additions & 4 deletions be/src/olap/task/index_builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -363,10 +363,14 @@ Status IndexBuilder::handle_single_rowset(RowsetMetaSharedPtr output_rowset_meta
auto column_name = inverted_index.columns[0];
auto column_idx = output_rowset_schema->field_index(column_name);
if (column_idx < 0) {
LOG(WARNING) << "referenced column was missing. "
<< "[column=" << column_name << " referenced_column=" << column_idx
<< "]";
continue;
column_idx =
output_rowset_schema->field_index(inverted_index.column_unique_ids[0]);
if (column_idx < 0) {
LOG(WARNING) << "referenced column was missing. "
<< "[column=" << column_name
<< " referenced_column=" << column_idx << "]";
continue;
}
}
auto column = output_rowset_schema->column(column_idx);
if (!InvertedIndexColumnWriter::check_support_inverted_index(column)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2135,6 +2135,7 @@ public int getAsInt() {
index.setIndexId(existedIdx.getIndexId());
index.setColumns(existedIdx.getColumns());
index.setProperties(existedIdx.getProperties());
index.setColumnUniqueIds(existedIdx.getColumnUniqueIds());
if (indexDef.getPartitionNames().isEmpty()) {
invertedIndexOnPartitions.put(index.getIndexId(), olapTable.getPartitionNames());
} else {
Expand Down Expand Up @@ -2735,6 +2736,7 @@ private boolean processAddIndex(CreateIndexClause alterClause, OlapTable olapTab
if (column != null) {
indexDef.checkColumn(column, olapTable.getKeysType(),
olapTable.getTableProperty().getEnableUniqueKeyMergeOnWrite());
indexDef.getColumnUniqueIds().add(column.getUniqueId());
} else {
throw new DdlException("index column does not exist in table. invalid column: " + col);
}
Expand All @@ -2745,6 +2747,7 @@ private boolean processAddIndex(CreateIndexClause alterClause, OlapTable olapTab
// so here update column name in CreateIndexClause after checkColumn for indexDef,
// there will use the column name in olapTable insead of the column name in CreateIndexClause.
alterIndex.setColumns(indexDef.getColumns());
alterIndex.setColumnUniqueIds(indexDef.getColumnUniqueIds());
newIndexes.add(alterIndex);
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void analyze(Analyzer analyzer) throws AnalysisException {
indexDef.analyze();
this.index = new Index(Env.getCurrentEnv().getNextId(), indexDef.getIndexName(),
indexDef.getColumns(), indexDef.getIndexType(),
indexDef.getProperties(), indexDef.getComment());
indexDef.getProperties(), indexDef.getComment(), indexDef.getColumnUniqueIds());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void analyze(Analyzer analyzer) throws AnalysisException {
indexDef.analyze();
this.index = new Index(Env.getCurrentEnv().getNextId(), indexDef.getIndexName(),
indexDef.getColumns(), indexDef.getIndexType(),
indexDef.getProperties(), indexDef.getComment());
indexDef.getProperties(), indexDef.getComment(), indexDef.getColumnUniqueIds());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,8 @@ public void analyze(Analyzer analyzer) throws UserException {
}
}
indexes.add(new Index(Env.getCurrentEnv().getNextId(), indexDef.getIndexName(), indexDef.getColumns(),
indexDef.getIndexType(), indexDef.getProperties(), indexDef.getComment()));
indexDef.getIndexType(), indexDef.getProperties(), indexDef.getComment(),
indexDef.getColumnUniqueIds()));
distinct.add(indexDef.getIndexName());
distinctCol.add(Pair.of(indexDef.getIndexType(),
indexDef.getColumns().stream().map(String::toUpperCase).collect(Collectors.toList())));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public class IndexDef {
private Map<String, String> properties;
private boolean isBuildDeferred = false;
private PartitionNames partitionNames;
private List<Integer> columnUniqueIds = Lists.newArrayList();

public static final String NGRAM_SIZE_KEY = "gram_size";
public static final String NGRAM_BF_SIZE_KEY = "bf_size";
Expand Down Expand Up @@ -196,6 +197,10 @@ public List<String> getPartitionNames() {
return partitionNames == null ? Lists.newArrayList() : partitionNames.getPartitionNames();
}

public List<Integer> getColumnUniqueIds() {
return columnUniqueIds;
}

public enum IndexType {
BITMAP,
INVERTED,
Expand Down
20 changes: 17 additions & 3 deletions fe/fe-core/src/main/java/org/apache/doris/catalog/Index.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@

/**
* Internal representation of index, including index type, name, columns and comments.
* This class will used in olaptable
* This class will be used in olap table
*/
public class Index implements Writable {
public static final int INDEX_ID_INIT_VALUE = -1;
Expand All @@ -65,15 +65,19 @@ public class Index implements Writable {
private Map<String, String> properties;
@SerializedName(value = "ct", alternate = {"comment"})
private String comment;
@SerializedName(value = "cui", alternate = {"columnUniqueIds"})
private List<Integer> columnUniqueIds;

public Index(long indexId, String indexName, List<String> columns,
IndexDef.IndexType indexType, Map<String, String> properties, String comment) {
IndexDef.IndexType indexType, Map<String, String> properties, String comment,
List<Integer> columnUniqueIds) {
this.indexId = indexId;
this.indexName = indexName;
this.columns = columns == null ? Lists.newArrayList() : Lists.newArrayList(columns);
this.indexType = indexType;
this.properties = properties == null ? Maps.newHashMap() : Maps.newHashMap(properties);
this.comment = comment;
this.columnUniqueIds = columnUniqueIds == null ? Lists.newArrayList() : Lists.newArrayList(columnUniqueIds);
if (indexType == IndexDef.IndexType.INVERTED) {
if (this.properties != null && !this.properties.isEmpty()) {
if (this.properties.containsKey(InvertedIndexUtil.INVERTED_INDEX_PARSER_KEY)) {
Expand All @@ -97,6 +101,7 @@ public Index() {
this.indexType = null;
this.properties = null;
this.comment = null;
this.columnUniqueIds = null;
}

public long getIndexId() {
Expand Down Expand Up @@ -186,6 +191,14 @@ public void setComment(String comment) {
this.comment = comment;
}

public List<Integer> getColumnUniqueIds() {
return columnUniqueIds;
}

public void setColumnUniqueIds(List<Integer> columnUniqueIds) {
this.columnUniqueIds = columnUniqueIds;
}

@Override
public void write(DataOutput out) throws IOException {
Text.writeString(out, GsonUtils.GSON.toJson(this));
Expand All @@ -203,7 +216,7 @@ public int hashCode() {

public Index clone() {
return new Index(indexId, indexName, new ArrayList<>(columns),
indexType, new HashMap<>(properties), comment);
indexType, new HashMap<>(properties), comment, columnUniqueIds);
}

@Override
Expand Down Expand Up @@ -247,6 +260,7 @@ public TOlapTableIndex toThrift() {
if (properties != null) {
tIndex.setProperties(properties);
}
tIndex.setColumnUniqueIds(columnUniqueIds);
return tIndex;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,13 @@ public void initSchemaColumnUniqueId() {
maxColUniqueId = Column.COLUMN_UNIQUE_ID_INIT_VALUE;
this.schema.forEach(column -> {
column.setUniqueId(incAndGetMaxColUniqueId());
this.indexes.forEach(index -> {
index.getColumns().forEach(col -> {
if (col.equalsIgnoreCase(column.getName())) {
index.getColumnUniqueIds().add(column.getUniqueId());
}
});
});
if (LOG.isDebugEnabled()) {
LOG.debug("indexId: {}, column:{}, uniqueId:{}",
indexId, column, column.getUniqueId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,6 @@ public IndexType getIndexType() {

public Index translateToCatalogStyle() {
return new Index(Env.getCurrentEnv().getNextId(), name, cols, indexType, properties,
comment);
comment, null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ int getCurrentEnvJournalVersion() {
}
OlapTable tbl = (OlapTable) table;
tbl.setIndexes(Lists.newArrayList(new Index(0, "index", Lists.newArrayList("col"),
IndexDef.IndexType.BITMAP, null, "xxxxxx")));
IndexDef.IndexType.BITMAP, null, "xxxxxx", Lists.newArrayList(1))));
System.out.println("orig table id: " + tbl.getId());

FastByteArrayOutputStream byteArrayOutputStream = new FastByteArrayOutputStream();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,18 @@ public class IndexesProcNodeTest {
public void testFetchResult() throws AnalysisException {
List<Index> indexes = new ArrayList<>();
Index indexBitmap = new Index(1, "bitmap_index", Lists.newArrayList("col_1"),
IndexType.BITMAP, null, "bitmap index on col_1");
IndexType.BITMAP, null, "bitmap index on col_1", Lists.newArrayList(1));
Map<String, String> invertedProperties = new HashMap<>();
invertedProperties.put("parser", "unicode");
Index indexInverted = new Index(2, "inverted_index", Lists.newArrayList("col_2"),
IndexType.INVERTED, invertedProperties, "inverted index on col_2");
IndexType.INVERTED, invertedProperties, "inverted index on col_2", Lists.newArrayList(2));
Index indexBf = new Index(3, "bloomfilter_index", Lists.newArrayList("col_3"),
IndexType.BLOOMFILTER, null, "bloomfilter index on col_3");
IndexType.BLOOMFILTER, null, "bloomfilter index on col_3", Lists.newArrayList(3));
Map<String, String> ngramProperties = new HashMap<>();
ngramProperties.put("gram_size", "3");
ngramProperties.put("bf_size", "256");
Index indexNgramBf = new Index(4, "ngram_bf_index", Lists.newArrayList("col_4"),
IndexType.NGRAM_BF, ngramProperties, "ngram_bf index on col_4");
IndexType.NGRAM_BF, ngramProperties, "ngram_bf index on col_4", Lists.newArrayList(4));
indexes.add(indexBitmap);
indexes.add(indexInverted);
indexes.add(indexBf);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public void testSerialization() throws IOException {
indexSchemaMap.put(tableId, fullSchema);

List<Index> indexes = Lists.newArrayList(
new Index(0, "index", Lists.newArrayList("testCol1"), IndexDef.IndexType.INVERTED, null, "xxxxxx"));
new Index(0, "index", Lists.newArrayList("testCol1"), IndexDef.IndexType.INVERTED, null, "xxxxxx", Lists.newArrayList(1)));

TableAddOrDropColumnsInfo tableAddOrDropColumnsInfo1 = new TableAddOrDropColumnsInfo("", dbId, tableId,
indexSchemaMap, indexes, jobId);
Expand Down
1 change: 1 addition & 0 deletions gensrc/thrift/Descriptors.thrift
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ struct TOlapTableIndex {
4: optional string comment
5: optional i64 index_id
6: optional map<string, string> properties
7: optional list<i32> column_unique_ids
}

struct TOlapTableIndexSchema {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
import org.codehaus.groovy.runtime.IOGroovyMethods

suite("test_index_change_on_renamed_column") {
def backendId_to_backendIP = [:]
def backendId_to_backendHttpPort = [:]
getBackendIpHttpPort(backendId_to_backendIP, backendId_to_backendHttpPort);

def timeout = 60000
def delta_time = 1000
def alter_res = "null"
Expand Down Expand Up @@ -67,7 +71,7 @@ suite("test_index_change_on_renamed_column") {
`id` INT COMMENT "",
`s` STRING COMMENT ""
)
DUPLICATE KEY(`id`) DISTRIBUTED BY HASH(`id`)
DUPLICATE KEY(`id`) DISTRIBUTED BY HASH(`id`) BUCKETS 1
PROPERTIES ( "replication_num" = "1" );
"""

Expand Down Expand Up @@ -101,6 +105,13 @@ suite("test_index_change_on_renamed_column") {
qt_select2 """ SELECT * FROM ${tableName} order by id; """
qt_select3 """ SELECT * FROM ${tableName} where s1 match 'welcome'; """

def tablets = sql_return_maparray """ show tablets from ${tableName}; """
String tablet_id = tablets[0].TabletId
String backend_id = tablets[0].BackendId
String ip = backendId_to_backendIP.get(backend_id)
String port = backendId_to_backendHttpPort.get(backend_id)
check_nested_index_file(ip, port, tablet_id, 3, 1, "V2")

// drop inverted index on renamed column
sql """ alter table ${tableName} drop index idx_s; """
wait_for_latest_op_on_table_finish(tableName, timeout)
Expand Down
Loading