[GitHub] [pinot] snleee commented on a diff in pull request #11540: Keep header parsing logic the same in default CSVRecordReader and the line iterator

[via GitHub]

snleee commented on code in PR #11540:
URL: https://github.com/apache/pinot/pull/11540#discussion_r1322249345


##########
pinot-plugins/pinot-input-format/pinot-csv/src/main/java/org/apache/pinot/plugin/inputformat/csv/CSVRecordReader.java:
##########
@@ -104,50 +103,42 @@ public void init(File dataFile, @Nullable Set<String> 
fieldsToRead, @Nullable Re
         }
       }
       char delimiter = config.getDelimiter();
-      format = format.withDelimiter(delimiter);
+      format = format.builder().setDelimiter(delimiter).build();
 
       if (config.isSkipUnParseableLines()) {
         _useLineIterator = true;
       }
 
-      String csvHeader = config.getHeader();
-      if (csvHeader == null) {
-        format = format.withHeader();
-      } else {
-        // do not validate header if using the line iterator
-        if (_useLineIterator) {
-          String[] header = StringUtils.split(csvHeader, delimiter);
-          setHeaderMap(header);
-          format = format.withHeader(header);
-          _isHeaderProvided = true;
-        } else {
-          // validate header for the delimiter before splitting
-          validateHeaderForDelimiter(delimiter, csvHeader, format);
-          format = format.withHeader(StringUtils.split(csvHeader, delimiter));
-        }
-      }
-      format = format.withSkipHeaderRecord(config.isSkipHeader());
+      _isHeaderProvided = config.getHeader() != null;
       _skipHeaderRecord = config.isSkipHeader();
-      format = format.withCommentMarker(config.getCommentMarker());
-      format = format.withEscape(config.getEscapeCharacter());
-      format = format.withIgnoreEmptyLines(config.isIgnoreEmptyLines());
-      format = 
format.withIgnoreSurroundingSpaces(config.isIgnoreSurroundingSpaces());
-      format = format.withQuote(config.getQuoteCharacter());
+      _format = format.builder()
+          .setHeader()
+          .setSkipHeaderRecord(config.isSkipHeader())
+          .setCommentMarker(config.getCommentMarker())
+          .setEscape(config.getEscapeCharacter())
+          .setIgnoreEmptyLines(config.isIgnoreEmptyLines())
+          .setIgnoreSurroundingSpaces(config.isIgnoreSurroundingSpaces())
+          .setQuote(config.getQuoteCharacter())
+          .build();
 
       if (config.getQuoteMode() != null) {
-        format = 
format.withQuoteMode(QuoteMode.valueOf(config.getQuoteMode()));
+        _format = 
_format.builder().setQuoteMode(QuoteMode.valueOf(config.getQuoteMode())).build();

Review Comment:
   then isn't the implementation incorrect? 
   
   We need to incrementally set more stuffs on top of the existing builder 
object instead of creating new builder multiple times?



-- 
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