wuchong commented on a change in pull request #13925:
URL: https://github.com/apache/flink/pull/13925#discussion_r517816286
##########
File path:
flink-formats/flink-csv/src/main/java/org/apache/flink/formats/csv/CsvFileSystemFormatFactory.java
##########
@@ -194,7 +186,12 @@ private CsvSchema buildCsvSchema(RowType rowType,
ReadableConfig options) {
final CsvRowDataSerializationSchema serializationSchema =
builder.build();
- return Optional.of((record, stream) ->
stream.write(serializationSchema.serialize(record)));
+ return Optional.of((record, stream) -> {
+ StringBuilder sb = new StringBuilder();
+ sb.append(new
String(serializationSchema.serialize(record)));
+ sb.append("\n");
+ stream.write(sb.toString().getBytes());
Review comment:
Why we hard code `\n` ? I think the purpose of this issue is removing
the "\n".
##########
File path:
flink-formats/flink-csv/src/main/java/org/apache/flink/formats/csv/CsvRowSerializationSchema.java
##########
@@ -86,8 +86,8 @@ private CsvRowSerializationSchema(
this.typeInfo = typeInfo;
this.runtimeConverter = createRowRuntimeConverter(typeInfo,
true);
this.csvMapper = new CsvMapper();
- this.csvSchema = csvSchema;
- this.objectWriter = csvMapper.writer(csvSchema);
+ this.csvSchema = csvSchema.withLineSeparator("");
Review comment:
ditto. I would recommend not changing the legacy csv format.
##########
File path: docs/dev/table/connectors/formats/csv.md
##########
@@ -95,18 +95,6 @@ Format Options
<td>String</td>
<td>Field delimiter character (<code>','</code> by default).</td>
</tr>
- <tr>
- <td><h5>csv.line-delimiter</h5></td>
- <td>optional</td>
- <td style="word-wrap: break-word;"><code>\n</code></td>
- <td>String</td>
- <td>Line delimiter, <code>\n</code> by default. Note the <code>\n</code>
and <code>\r</code> are invisible special characters, you have to use unicode
to specify them in plain SQL.
- <ul>
- <li>e.g. <code>'csv.line-delimiter' = U&'\000D'</code> specifies
the to use carriage return <code>\r</code> as line delimiter.</li>
- <li>e.g. <code>'csv.line-delimiter' = U&'\000A'</code> specifies
the to use line feed <code>\n</code> as line delimiter.</li>
- </ul>
Review comment:
Move this notes to {{field-delimiter}} option.
##########
File path:
flink-formats/flink-csv/src/main/java/org/apache/flink/formats/csv/CsvRowFormatFactory.java
##########
@@ -101,9 +100,6 @@ public CsvRowFormatFactory() {
descriptorProperties.getOptionalCharacter(CsvValidator.FORMAT_FIELD_DELIMITER)
.ifPresent(schemaBuilder::setFieldDelimiter);
-
descriptorProperties.getOptionalString(CsvValidator.FORMAT_LINE_DELIMITER)
- .ifPresent(schemaBuilder::setLineDelimiter);
Review comment:
I would recommend not changing the legacy csv format.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
