Copilot commented on code in PR #6103:
URL: https://github.com/apache/paimon/pull/6103#discussion_r2292747886
##########
paimon-spark/paimon-spark-ut/src/test/java/org/apache/paimon/spark/SparkWriteITCase.java:
##########
@@ -682,7 +682,7 @@ public void testChangelogFileSuffixName() throws Exception {
.filter(name -> name.contains("changelog-"))
.collect(Collectors.toList());
String defaultExtension = "." + "parquet";
- String newExtension = "." + "zstd" + "." + "parquet";
+ String newExtension = "." + "zst" + "." + "parquet";
Review Comment:
The extension string concatenation is inconsistent with the comment that
mentions ".zstd.parquet". The comment states "two data file end with
.zstd.parquet" but the code uses "zst" which is the file extension for ZSTD
compression, not the compression type name.
```suggestion
String newExtension = "." + "zstd" + "." + "parquet";
```
##########
paimon-format/src/main/java/org/apache/paimon/format/BaseTextFileWriter.java:
##########
@@ -52,6 +66,7 @@ protected BaseTextFileWriter(PositionOutputStream
outputStream, RowType rowType)
public void close() throws IOException {
writer.flush();
writer.close();
+ compressedStream.close();
Review Comment:
The compressedStream.close() is called after writer.close(), but the writer
is wrapping the compressedStream. This could lead to the stream being closed
twice. The writer.close() should already close the underlying stream, making
the explicit compressedStream.close() redundant and potentially problematic.
```suggestion
```
##########
paimon-format/src/main/java/org/apache/paimon/format/BaseTextFileReader.java:
##########
@@ -91,6 +97,7 @@ public FileRecordIterator<InternalRow> readBatch() throws
IOException {
public void close() throws IOException {
if (!readerClosed && bufferedReader != null) {
bufferedReader.close();
+ decompressedStream.close();
Review Comment:
Similar to the writer, calling decompressedStream.close() after
bufferedReader.close() may cause the stream to be closed twice since
BufferedReader.close() should close the underlying stream. This could lead to
exceptions or resource leaks.
```suggestion
// No need to close decompressedStream explicitly;
bufferedReader.close() will do it.
```
##########
paimon-spark/paimon-spark-ut/src/test/java/org/apache/paimon/spark/SparkWriteITCase.java:
##########
@@ -638,7 +638,7 @@ public void testDataFileSuffixName() {
Assertions.assertEquals(4, files.size());
String defaultExtension = "." + "parquet";
- String newExtension = "." + "zstd" + "." + "parquet";
+ String newExtension = "." + "zst" + "." + "parquet";
Review Comment:
The extension string concatenation is inconsistent with the comment that
mentions ".zstd.parquet". The comment states "two data file end with
.zstd.parquet" but the code uses "zst" which is the file extension for ZSTD
compression, not the compression type name.
```suggestion
String newExtension = "." + "zstd" + "." + "parquet";
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
