This is an automated email from the ASF dual-hosted git repository.
ankitsultana pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 747e34dede Allow String / numeric data type for deleteRecordColumn
config (#12222)
747e34dede is described below
commit 747e34dedef7b4a318cb0e10e7f3c85b0b038036
Author: Pratik Tibrewal <[email protected]>
AuthorDate: Wed Jan 10 12:12:11 2024 +0530
Allow String / numeric data type for deleteRecordColumn config (#12222)
* Remove enforcement of BOOLEAN data type from deleteRecordColumn
* add string and numeric type for boolean columns
* addressed comments
* improve error messages
---
.../segment/local/utils/TableConfigUtils.java | 10 +++-
.../segment/local/utils/TableConfigUtilsTest.java | 58 +++++++++++++++++++---
2 files changed, 59 insertions(+), 9 deletions(-)
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
index 93aa057ecd..34d1e90fdb 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
@@ -731,9 +731,15 @@ public final class TableConfigUtils {
String deleteRecordColumn = upsertConfig.getDeleteRecordColumn();
if (deleteRecordColumn != null) {
FieldSpec fieldSpec = schema.getFieldSpecFor(deleteRecordColumn);
+ Preconditions.checkState(fieldSpec != null,
+ String.format("Column %s specified in deleteRecordColumn does not
exist", deleteRecordColumn));
+ Preconditions.checkState(fieldSpec.isSingleValueField(),
+ String.format("The deleteRecordColumn - %s must be a single-valued
column", deleteRecordColumn));
+ DataType dataType = fieldSpec.getDataType();
Preconditions.checkState(
- fieldSpec != null && fieldSpec.isSingleValueField() &&
fieldSpec.getDataType() == DataType.BOOLEAN,
- "The delete record column must be a single-valued BOOLEAN column");
+ dataType == DataType.BOOLEAN || dataType == DataType.STRING ||
dataType.isNumeric(),
+ String.format("The deleteRecordColumn - %s must be of type: String
/ Boolean / Numeric",
+ deleteRecordColumn));
}
String outOfOrderRecordColumn = upsertConfig.getOutOfOrderRecordColumn();
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java
index 1cc6f3d2b5..22aff329d7 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java
@@ -1746,26 +1746,30 @@ public class TableConfigUtilsTest {
}
// Table upsert with delete column
- String incorrectTypeDelCol = "incorrectTypeDeleteCol";
+ String stringTypeDelCol = "stringTypeDelCol";
String delCol = "myDelCol";
+ String mvCol = "mvCol";
+ String timestampCol = "timestampCol";
+ String invalidCol = "invalidCol";
schema = new
Schema.SchemaBuilder().setSchemaName(TABLE_NAME).setPrimaryKeyColumns(Lists.newArrayList("myPkCol"))
.addSingleValueDimension("myCol", FieldSpec.DataType.STRING)
- .addSingleValueDimension(incorrectTypeDelCol,
FieldSpec.DataType.STRING)
- .addSingleValueDimension(delCol, FieldSpec.DataType.BOOLEAN).build();
+ .addSingleValueDimension(stringTypeDelCol, FieldSpec.DataType.STRING)
+ .addSingleValueDimension(delCol, FieldSpec.DataType.BOOLEAN)
+ .addSingleValueDimension(timestampCol, FieldSpec.DataType.TIMESTAMP)
+ .addMultiValueDimension(mvCol, FieldSpec.DataType.STRING).build();
streamConfigs = getStreamConfigs();
streamConfigs.put("stream.kafka.consumer.type", "simple");
upsertConfig = new UpsertConfig(UpsertConfig.Mode.FULL);
- upsertConfig.setDeleteRecordColumn(incorrectTypeDelCol);
+ upsertConfig.setDeleteRecordColumn(stringTypeDelCol);
tableConfig = new
TableConfigBuilder(TableType.REALTIME).setTableName(TABLE_NAME).setStreamConfigs(streamConfigs)
.setUpsertConfig(upsertConfig)
.setRoutingConfig(new RoutingConfig(null, null,
RoutingConfig.STRICT_REPLICA_GROUP_INSTANCE_SELECTOR_TYPE))
.build();
try {
TableConfigUtils.validateUpsertAndDedupConfig(tableConfig, schema);
- Assert.fail("Invalid delete column type (string) should have failed
table creation");
} catch (IllegalStateException e) {
- Assert.assertEquals(e.getMessage(), "The delete record column must be a
single-valued BOOLEAN column");
+ Assert.fail("Shouldn't fail table creation when delete column type is
single-valued.");
}
upsertConfig = new UpsertConfig(UpsertConfig.Mode.FULL);
@@ -1777,7 +1781,47 @@ public class TableConfigUtilsTest {
try {
TableConfigUtils.validateUpsertAndDedupConfig(tableConfig, schema);
} catch (IllegalStateException e) {
- Assert.fail("Shouldn't fail table creation when delete column type is
boolean.");
+ Assert.fail("Shouldn't fail table creation when delete column type is
single-valued.");
+ }
+
+ upsertConfig = new UpsertConfig(UpsertConfig.Mode.FULL);
+ upsertConfig.setDeleteRecordColumn(timestampCol);
+ tableConfig = new
TableConfigBuilder(TableType.REALTIME).setTableName(TABLE_NAME).setStreamConfigs(streamConfigs)
+ .setUpsertConfig(upsertConfig)
+ .setRoutingConfig(new RoutingConfig(null, null,
RoutingConfig.STRICT_REPLICA_GROUP_INSTANCE_SELECTOR_TYPE))
+ .build();
+ try {
+ TableConfigUtils.validateUpsertAndDedupConfig(tableConfig, schema);
+ Assert.fail("Should have failed table creation when delete column type
is timestamp.");
+ } catch (IllegalStateException e) {
+ Assert.assertEquals(e.getMessage(),
+ "The deleteRecordColumn - timestampCol must be of type: String /
Boolean / Numeric");
+ }
+
+ upsertConfig = new UpsertConfig(UpsertConfig.Mode.FULL);
+ upsertConfig.setDeleteRecordColumn(invalidCol);
+ tableConfig = new
TableConfigBuilder(TableType.REALTIME).setTableName(TABLE_NAME).setStreamConfigs(streamConfigs)
+ .setUpsertConfig(upsertConfig)
+ .setRoutingConfig(new RoutingConfig(null, null,
RoutingConfig.STRICT_REPLICA_GROUP_INSTANCE_SELECTOR_TYPE))
+ .build();
+ try {
+ TableConfigUtils.validateUpsertAndDedupConfig(tableConfig, schema);
+ Assert.fail("Should have failed table creation when invalid delete
column entered.");
+ } catch (IllegalStateException e) {
+ Assert.assertEquals(e.getMessage(), "Column invalidCol specified in
deleteRecordColumn does not exist");
+ }
+
+ upsertConfig = new UpsertConfig(UpsertConfig.Mode.FULL);
+ upsertConfig.setDeleteRecordColumn(mvCol);
+ tableConfig = new
TableConfigBuilder(TableType.REALTIME).setTableName(TABLE_NAME).setStreamConfigs(streamConfigs)
+ .setUpsertConfig(upsertConfig)
+ .setRoutingConfig(new RoutingConfig(null, null,
RoutingConfig.STRICT_REPLICA_GROUP_INSTANCE_SELECTOR_TYPE))
+ .build();
+ try {
+ TableConfigUtils.validateUpsertAndDedupConfig(tableConfig, schema);
+ Assert.fail("Should have failed table creation when delete column type
is multi-valued.");
+ } catch (IllegalStateException e) {
+ Assert.assertEquals(e.getMessage(), "The deleteRecordColumn - mvCol must
be a single-valued column");
}
// upsert deleted-keys-ttl configs with no deleted column
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
