You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/01/15 02:14:48 UTC
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #7906: Add global strategy for partial upsert
Jackie-Jiang commented on a change in pull request #7906:
URL: https://github.com/apache/pinot/pull/7906#discussion_r785256245
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/PartialUpsertHandler.java
##########
@@ -48,13 +49,27 @@
private final String _tableNameWithType;
private boolean _allSegmentsLoaded;
- public PartialUpsertHandler(HelixManager helixManager, String tableNameWithType,
- Map<String, UpsertConfig.Strategy> partialUpsertStrategies) {
+ public PartialUpsertHandler(HelixManager helixManager, String tableNameWithType, Schema schema,
+ Map<String, UpsertConfig.Strategy> partialUpsertStrategies, UpsertConfig.Strategy defaultPartialUpsertStrategy) {
_helixManager = helixManager;
_tableNameWithType = tableNameWithType;
for (Map.Entry<String, UpsertConfig.Strategy> entry : partialUpsertStrategies.entrySet()) {
_column2Mergers.put(entry.getKey(), PartialUpsertMergerFactory.getMerger(entry.getValue()));
}
+
+ if (schema != null) {
+ for (String dimensionName : schema.getDimensionNames()) {
Review comment:
We should probably include all physical columns (including date time columns) except for primary key columns and comparison column (main time column if no comparison column is configured)
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/PartialUpsertHandler.java
##########
@@ -48,13 +49,27 @@
private final String _tableNameWithType;
private boolean _allSegmentsLoaded;
- public PartialUpsertHandler(HelixManager helixManager, String tableNameWithType,
- Map<String, UpsertConfig.Strategy> partialUpsertStrategies) {
+ public PartialUpsertHandler(HelixManager helixManager, String tableNameWithType, Schema schema,
+ Map<String, UpsertConfig.Strategy> partialUpsertStrategies, UpsertConfig.Strategy defaultPartialUpsertStrategy) {
_helixManager = helixManager;
_tableNameWithType = tableNameWithType;
for (Map.Entry<String, UpsertConfig.Strategy> entry : partialUpsertStrategies.entrySet()) {
_column2Mergers.put(entry.getKey(), PartialUpsertMergerFactory.getMerger(entry.getValue()));
}
+
+ if (schema != null) {
Review comment:
Schema should never be `null` here
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org