Jackie-Jiang commented on a change in pull request #6096:
URL: https://github.com/apache/incubator-pinot/pull/6096#discussion_r499105212
##########
File path:
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/UpsertConfig.java
##########
@@ -27,44 +27,22 @@
public class UpsertConfig extends BaseJsonConfig {
- // names of the columns that used as primary keys of an upsert table
- private final List<String> _primaryKeyColumns;
- // name of the column that we are going to store the offset value to
- private final String _offsetColumn;
- // name of the virtual column that we are going to store the validFrom value
to
- private final String _validFromColumn;
- // name of the virtual column that we are going to store the validUntil
value to
- private final String _validUntilColumn;
+ public static final String MODE_KEY = "mode";
- @JsonCreator
- public UpsertConfig(@JsonProperty(value = "primaryKeyColumns") List<String>
primaryKeyColumns,
- @JsonProperty(value = "offsetColumn") String offsetColumn,
- @JsonProperty(value = "validFromColumn") String validFromColumn,
- @JsonProperty(value = "validUntilColumn") String validUntilColumn) {
- Preconditions.checkArgument(primaryKeyColumns != null &&
primaryKeyColumns.size() == 1,
- "'primaryKeyColumns' must be configured with exact one column");
- Preconditions.checkArgument(StringUtils.isNotEmpty(offsetColumn),
"'offsetColumn' must be configured");
- Preconditions.checkArgument(StringUtils.isNotEmpty(validFromColumn),
"'validFromColumn' must be configured");
- Preconditions.checkArgument(StringUtils.isNotEmpty(validUntilColumn),
"'validUntilColumn' must be configured");
- _primaryKeyColumns = primaryKeyColumns;
- _offsetColumn = offsetColumn;
- _validFromColumn = validFromColumn;
- _validUntilColumn = validUntilColumn;
+ public enum Mode {
+ FULL, PARTIAL, NONE
Review comment:
Recommend not putting `NONE` as a mode. Upsert is disabled when the
`UpsertConfig` is `null` in the table config (more intuitive IMO)
##########
File path: pinot-spi/src/main/java/org/apache/pinot/spi/data/Schema.java
##########
@@ -66,6 +66,9 @@
private TimeFieldSpec _timeFieldSpec;
private final List<DateTimeFieldSpec> _dateTimeFieldSpecs = new
ArrayList<>();
private final List<ComplexFieldSpec> _complexFieldSpecs = new ArrayList<>();
+ // names of the columns that used as primary keys
+ // TODO: add validation checks like duplicate columns and use of time column
+ private List<String> _primaryKeyColumns;
Review comment:
This should not be part of the schema, but the upsert config
##########
File path:
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/TableConfig.java
##########
@@ -265,4 +266,9 @@ public void setIngestionConfig(IngestionConfig
ingestionConfig) {
public void setTierConfigsList(List<TierConfig> tierConfigsList) {
_tierConfigsList = tierConfigsList;
}
+
+ @JsonIgnore
+ public UpsertConfig.Mode getUpsertMode() {
Review comment:
Not sure if this method is needed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
