Skip to content

Conversation

@alicela
Copy link
Contributor

@alicela alicela commented Jul 17, 2025

No description provided.

}
//Merge header and data
try (var output = Files.newOutputStream(tmpOutputFile, StandardOpenOption.APPEND);
var input = Files.newInputStream(Path.of(tmpOutputFile.toAbsolutePath() + "data"))) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer using Path.resolve() for path construction over string concatenation

Map<String,String> columns = SqlUtils.getColumnTypes(getDatabase(), datasetName);

if(columnNames.isEmpty()){
if(columns.isEmpty()){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really a return we want if columns is empty? Maybe a continue is better to not interrupt the writing process for next datasets.

input.transferTo(output);
}
Files.deleteIfExists(Path.of(tmpOutputFile + "data"));
String content = Files.readString(tmpOutputFile);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am afraid that we could have performance issues here. If i am not mistaken Files.readString() will put all the content of the file in RAM, which can be a problem for big files.

Suggestion : deport this logic in a method where we use stream to read line by line the file, something like :

Path tempFile = Files.createTempFile("csv_clean_", ".tmp");

try (BufferedReader reader = Files.newBufferedReader(file);
     BufferedWriter writer = Files.newBufferedWriter(tempFile, StandardOpenOption.WRITE)) {

    String line;
    while ((line = reader.readLine()) != null) {
        writer.write(line.replace(NULL_MARKER, "\"\""));
        writer.newLine();
    }
}

Files.move(tempFile, file, java.nio.file.StandardCopyOption.REPLACE_EXISTING);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants