rohityadav1993 commented on code in PR #13837:
URL: https://github.com/apache/pinot/pull/13837#discussion_r1733370718


##########
pinot-connectors/pinot-flink-connector/src/main/java/org/apache/pinot/connector/flink/sink/PinotSinkFunction.java:
##########
@@ -76,13 +84,25 @@ public PinotSinkFunction(PinotGenericRowConverter<T> 
recordConverter, TableConfi
     _schema = schema;
     _segmentFlushMaxNumRecords = segmentFlushMaxNumRecords;
     _executorPoolSize = executorPoolSize;
+    _segmentNamePrefix = DEFAULT_UPLOADED_REALTIME_SEGMENT_PREFIX;
+  }
+
+  public PinotSinkFunction(PinotGenericRowConverter<T> recordConverter, 
TableConfig tableConfig, Schema schema,
+      long segmentFlushMaxNumRecords, int executorPoolSize, String 
segmentNamePrefix,
+      @Nullable Long segmentUploadTimeMs) {
+    this(recordConverter, tableConfig, schema, segmentFlushMaxNumRecords, 
executorPoolSize);
+    if (!segmentNamePrefix.isBlank()) {

Review Comment:
   It would depend on the segmentNameGenerator property if the prefix is 
needed, even though configured. But rethinking this, there are two ways to 
configure this:
   - In table config as batchIngestionConfig
   - In code via PinotSinkFunction config -> SegmentGenerator config
   
   Right now, latter will always take precedence with a default value. 
Refactoring so that default value is used when neither is configured otherwise 



-- 
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-unsubscr...@pinot.apache.org

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

Reply via email to