swaminathanmanish commented on code in PR #13540: URL: https://github.com/apache/pinot/pull/13540#discussion_r1668332686
########## pinot-core/src/main/java/org/apache/pinot/core/segment/processing/genericrow/GenericRowFileManager.java: ########## @@ -99,12 +101,18 @@ public void closeFileWriter() /** * Returns the file reader. Creates one if not exists. */ - public GenericRowFileReader getFileReader() + public GenericRowReader getFileReader() throws IOException { if (_fileReader == null) { Preconditions.checkState(_offsetFile.exists(), "Record offset file: %s does not exist", _offsetFile); Preconditions.checkState(_dataFile.exists(), "Record data file: %s does not exist", _dataFile); - _fileReader = new GenericRowFileReader(_offsetFile, _dataFile, _fieldSpecs, _includeNullFields, _numSortFields); + Map<String, Object> params = new HashMap<>(); Review Comment: Would these params be common to all GenericRowReaders ? if so, we can create a class to hold all these params, instead of Map<String, Object>? ########## pinot-core/src/main/java/org/apache/pinot/core/segment/processing/genericrow/GenericRowFileReader.java: ########## @@ -85,7 +85,7 @@ public int compare(int rowId1, int rowId2) { /** * Returns a record reader for the rows within the file. Records are sorted if sort order is configured. */ - public GenericRowFileRecordReader getRecordReader() { + public GenericRowMapperOutputRecordReader getRecordReader() { Review Comment: Do you plan to use this interface only for SegmentMapper and not for anything else? If thats the case, you can move it to mapper package, so that its clear that this abstraction is only for Segment mapper. -- 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