Jackie-Jiang commented on a change in pull request #7222:
URL: https://github.com/apache/pinot/pull/7222#discussion_r681344491
##########
File path:
pinot-plugins/pinot-batch-ingestion/pinot-batch-ingestion-common/src/main/java/org/apache/pinot/plugin/ingestion/batch/common/SegmentGenerationTaskRunner.java
##########
@@ -149,6 +155,9 @@ private SegmentNameGenerator getSegmentNameGenerator() {
Boolean.parseBoolean(segmentNameGeneratorConfigs.get(EXCLUDE_SEQUENCE_ID)),
IngestionConfigUtils.getBatchSegmentIngestionType(tableConfig),
IngestionConfigUtils.getBatchSegmentIngestionFrequency(tableConfig),
dateTimeFormatSpec);
+ case INPUT_FILE_SEGMENT_NAME_GENERATOR:
+ return new
InputFileSegmentNameGenerator(segmentNameGeneratorConfigs.get(FILE_PATH_PATTERN),
Review comment:
My concern here is that in several use cases (realtime to offline,
segment merge etc.), the segments are not generated from input files, but from
other segments. Modeling input file path as a parameter in
`SegmentNameGenerator.generateSegmentName()` can cause issues for these cases.
Seems generating segment name from input path only apply to
`SegmentGenerationTaskRunner` so I would suggest resolving the segment name
there.
But this cannot solve the local copy change path problem. Another approach
would be directly setting the table name within the `SegmentNameGeneratorSpec`
before copying the remote file to local
--
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
