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

[BugFix] List partition values should not contain NULL partition value if this column is not nullable (backport #51086) (backport #51147) #51163

Closed
wants to merge 1 commit into from
Closed

[BugFix] List partition values should not contain NULL partition value if this column is not nullable (backport #51086) (backport #51147) #51163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 10 additions & 0 deletions fe/fe-core/src/main/java/com/starrocks/catalog/OlapTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1595,6 +1595,16 @@ public Collection<Partition> getPartitions() {
return idToPartition.values();
}

/**
* Return all visible partitions except shadow partitions.
*/
public List<Partition> getVisiblePartitions() {
return nameToPartition.entrySet().stream()
.filter(e -> !e.getKey().startsWith(ExpressionRangePartitionInfo.SHADOW_PARTITION_PREFIX))
.map(e -> e.getValue())
.collect(Collectors.toList());
}

public List<Partition> getNonEmptyPartitions() {
return idToPartition.values().stream().filter(Partition::hasData).collect(Collectors.toList());
}
Expand Down
4 changes: 2 additions & 2 deletions fe/fe-core/src/main/java/com/starrocks/planner/OlapScanNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,7 @@ protected String getNodeExplainString(String prefix, TExplainLevel detailLevel)

if (detailLevel != TExplainLevel.VERBOSE) {
output.append(prefix).append(String.format("partitions=%s/%s\n", selectedPartitionNum,
olapTable.getPartitions().size()));
olapTable.getVisiblePartitionNames().size()));

String indexName = olapTable.getIndexNameById(selectedIndexId);
output.append(prefix).append(String.format("rollup: %s\n", indexName));
Expand All @@ -809,7 +809,7 @@ protected String getNodeExplainString(String prefix, TExplainLevel detailLevel)
output.append(prefix).append(String.format(
"partitionsRatio=%s/%s",
selectedPartitionNum,
olapTable.getPartitions().size())).append(", ")
olapTable.getVisiblePartitionNames().size())).append(", ")
.append(String.format("tabletsRatio=%s/%s", selectedTabletsNum, totalTabletsNum)).append("\n");

if (scanTabletIds.size() > 10) {
Expand Down
2 changes: 1 addition & 1 deletion fe/fe-core/src/main/java/com/starrocks/sql/Explain.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ public OperatorStr visitPhysicalOlapScan(OptExpression optExpression, OperatorPr
String partitionAndBucketInfo = "partitionRatio: " +
scan.getSelectedPartitionId().size() +
"/" +
((OlapTable) scan.getTable()).getPartitions().size() +
((OlapTable) scan.getTable()).getVisiblePartitionNames().size() +
", tabletRatio: " +
scan.getSelectedTabletId().size() +
"/" +
Expand Down
35 changes: 35 additions & 0 deletions fe/fe-core/src/main/java/com/starrocks/sql/ast/ListPartitionDesc.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ public void analyze(List<ColumnDef> columnDefs, Map<String, String> tablePropert
this.analyzeSingleListPartition(tableProperties, columnDefList);
// analyze multi list partition
this.analyzeMultiListPartition(tableProperties, columnDefList);
// list partition values should not contain NULL partition value if this column is not nullable.
this.postAnalyzePartitionColumns(columnDefList);
}

public List<ColumnDef> analyzePartitionColumns(List<ColumnDef> columnDefs) throws AnalysisException {
Expand Down Expand Up @@ -141,6 +143,39 @@ public List<ColumnDef> analyzePartitionColumns(List<ColumnDef> columnDefs) throw
return partitionColumns;
}

private void postAnalyzePartitionColumns(List<ColumnDef> columnDefs) throws AnalysisException {
// list partition values should not contain NULL partition value if this column is not nullable.
int partitionColSize = columnDefs.size();
for (int i = 0; i < columnDefs.size(); i++) {
ColumnDef columnDef = columnDefs.get(i);
if (columnDef.isAllowNull()) {
continue;
}
String partitionCol = columnDef.getName();
for (SingleItemListPartitionDesc desc : singleListPartitionDescs) {
for (LiteralExpr literalExpr : desc.getLiteralExprValues()) {
if (literalExpr.isNullable()) {
throw new AnalysisException("Partition column[" + partitionCol + "] could not be null but " +
"contains null value in partition[" + desc.getPartitionName() + "]");
}
}
}
for (MultiItemListPartitionDesc desc : multiListPartitionDescs) {
for (List<LiteralExpr> literalExprs : desc.getMultiLiteralExprValues()) {
if (literalExprs.size() != partitionColSize) {
throw new AnalysisException("Partition column[" + partitionCol + "] size should be equal to " +
"partition column size but contains " + literalExprs.size() + " values in partition[" +
desc.getPartitionName() + "]");
}
if (literalExprs.get(i).isNullable()) {
throw new AnalysisException("Partition column[" + partitionCol + "] could not be null but " +
"contains null value in partition[" + desc.getPartitionName() + "]");
}
}
}
}
}

public void analyzeExternalPartitionColumns(List<ColumnDef> columnDefs, String engineName) {
if (this.partitionColNames == null || this.partitionColNames.isEmpty()) {
throw new SemanticException("No partition columns.");
Expand Down
2 changes: 1 addition & 1 deletion fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rewrite/OptOlapPartitionPruner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public static LogicalOlapScanOperator prunePartitions(LogicalOlapScanOperator lo

if (selectedPartitionIds == null) {
selectedPartitionIds =
table.getPartitions().stream().filter(Partition::hasData).map(Partition::getId).collect(
table.getVisiblePartitions().stream().filter(Partition::hasData).map(Partition::getId).collect(
Collectors.toList());
// some test cases need to perceive partitions pruned, so we can not filter empty partitions.
} else {
Expand Down
1 change: 0 additions & 1 deletion fe/fe-core/src/test/java/com/starrocks/analysis/CreateMaterializedViewTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3753,7 +3753,6 @@ public void testCreateMaterializedViewOnListPartitionTables2() throws Exception
starRocksAssert.dropTable("list_partition_tbl1");
}


@Test
public void testCreateMaterializedViewOnListPartitionTables3() {
String createSQL = "CREATE TABLE test.list_partition_tbl1 (\n" +
Expand Down
59 changes: 59 additions & 0 deletions fe/fe-core/src/test/java/com/starrocks/catalog/CreateTableTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
package com.starrocks.catalog;

import com.starrocks.alter.AlterJobException;
import com.starrocks.analysis.LiteralExpr;
import com.starrocks.catalog.constraint.UniqueConstraint;
import com.starrocks.common.AnalysisException;
import com.starrocks.common.Config;
Expand Down Expand Up @@ -70,6 +71,7 @@

import java.util.ArrayList;
import java.util.List;
import java.util.Map;

public class CreateTableTest {
private static ConnectContext connectContext;
Expand All @@ -93,6 +95,7 @@ public static void beforeClass() throws Exception {
GlobalStateMgr.getCurrentState().getMetadata().createDb(createDbStmt.getFullDbName());

UtFrameUtils.setUpForPersistTest();
starRocksAssert.useDatabase("test");
}

private static void createTable(String sql) throws Exception {
Expand Down Expand Up @@ -2074,4 +2077,60 @@ public void testDefaultValueHasEscapeString() throws Exception {
starRocksAssert.dropTable("news_rt");
starRocksAssert.withTable(createTableSql);
}

@Test
public void testCreateTableWithNullableColumns1() throws Exception {
String createSQL = "CREATE TABLE list_partition_tbl1 (\n" +
" id BIGINT,\n" +
" age SMALLINT,\n" +
" dt VARCHAR(10),\n" +
" province VARCHAR(64) \n" +
")\n" +
"DUPLICATE KEY(id)\n" +
"PARTITION BY LIST (province) (\n" +
" PARTITION p1 VALUES IN ((NULL),(\"chongqing\")) ,\n" +
" PARTITION p2 VALUES IN ((\"guangdong\")) \n" +
")\n" +
"DISTRIBUTED BY RANDOM\n" +
"PROPERTIES (\n" +
"\"replication_num\" = \"1\"\n" +
");";
starRocksAssert.withTable(createSQL);
Database db = GlobalStateMgr.getCurrentState().getLocalMetastore().getDb("test");
OlapTable table = (OlapTable) GlobalStateMgr.getCurrentState().getLocalMetastore().getTable(db.getFullName(),
"list_partition_tbl1");
PartitionInfo info = table.getPartitionInfo();
Assert.assertTrue(info.isListPartition());
ListPartitionInfo listPartitionInfo = (ListPartitionInfo) info;
Map<Long, List<List<LiteralExpr>>> long2Literal = listPartitionInfo.getMultiLiteralExprValues();
Assert.assertEquals(2, long2Literal.size());
}

@Test
public void testCreateTableWithNullableColumns2() {
String createSQL = "\n" +
"CREATE TABLE t3 (\n" +
" dt date,\n" +
" city varchar(20),\n" +
" name varchar(20),\n" +
" num int\n" +
") ENGINE=OLAP\n" +
"PRIMARY KEY(dt, city, name)\n" +
"PARTITION BY LIST (dt) (\n" +
" PARTITION p1 VALUES IN ((NULL), (\"2022-04-01\")),\n" +
" PARTITION p2 VALUES IN ((\"2022-04-02\")),\n" +
" PARTITION p3 VALUES IN ((\"2022-04-03\"))\n" +
")\n" +
"DISTRIBUTED BY HASH(dt) BUCKETS 3\n" +
"PROPERTIES (\n" +
" \"replication_num\" = \"1\"\n" +
");";
try {
starRocksAssert.withTable(createSQL);
Assert.fail();
} catch (Exception e) {
Assert.assertTrue(e.getMessage().contains("Partition column[dt] could not be null but contains null " +
"value in partition[p1]."));
}
}
}
Loading
Loading