Skip to content

Commit 84d33ab

Browse files
atlan-owenclaude
andcommitted
fix(csv): use configured field separator in parallel chunk path (CSA-484)
CSVReader.streamRows() hardcoded comma in chunk write/read, crashing on non-comma-delimited files with CsvParseException. Promote fieldSeparator to a class field and use it at lines 190, 202, 219. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: atlan-owen <owen.price@atlan.com>
1 parent 9e7ce57 commit 84d33ab

2 files changed

Lines changed: 110 additions & 4 deletions

File tree

package-toolkit/runtime/src/main/kotlin/com/atlan/pkg/serde/csv/CSVReader.kt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ class CSVReader
5959
private val atlanTagHandling: AtlanTagHandling = AtlanTagHandling.REPLACE,
6060
private val creationHandling: AssetCreationHandling = AssetCreationHandling.FULL,
6161
private val tableViewAgnostic: Boolean = false,
62-
fieldSeparator: Char = ',',
62+
private val fieldSeparator: Char = ',',
6363
private val linkIdempotency: LinkIdempotencyInvariant = LinkIdempotencyInvariant.URL,
6464
) : Closeable {
6565
private val reader: CsvReader<CsvRecord>
@@ -187,7 +187,7 @@ class CSVReader
187187
val chunkSize = totalRowCount / parallelism
188188
val currentRecordCount = AtomicLong(0)
189189
var chunkPath = Files.createTempFile("chunk_0_", ".csv")
190-
var writer = CSVWriter(chunkPath.toString(), ',')
190+
var writer = CSVWriter(chunkPath.toString(), fieldSeparator)
191191
reader.stream().skip(1).forEach { row: CsvRecord ->
192192
// Split the original file up into multiple smaller files for parallel-processing
193193
if (rowToAsset.includeRow(row.fields, header, typeIdx, qualifiedNameIdx)) {
@@ -199,7 +199,7 @@ class CSVReader
199199
csvChunkFiles.add(chunkPath)
200200
currentRecordCount.set(0)
201201
chunkPath = Files.createTempFile("chunk_${csvChunkFiles.size}_", ".csv")
202-
writer = CSVWriter(chunkPath.toString(), ',')
202+
writer = CSVWriter(chunkPath.toString(), fieldSeparator)
203203
}
204204
}
205205
}
@@ -216,7 +216,7 @@ class CSVReader
216216
val reader =
217217
CsvReader
218218
.builder()
219-
.fieldSeparator(',')
219+
.fieldSeparator(fieldSeparator)
220220
.quoteCharacter('"')
221221
.skipEmptyLines(true)
222222
.extraFieldStrategy(FieldMismatchStrategy.STRICT)
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/* SPDX-License-Identifier: Apache-2.0
2+
Copyright 2023 Atlan Pte. Ltd. */
3+
package serde
4+
5+
import de.siegmar.fastcsv.reader.CsvParseException
6+
import de.siegmar.fastcsv.reader.CsvReader
7+
import de.siegmar.fastcsv.reader.CsvRecord
8+
import de.siegmar.fastcsv.reader.FieldMismatchStrategy
9+
import de.siegmar.fastcsv.writer.CsvWriter
10+
import de.siegmar.fastcsv.writer.LineDelimiter
11+
import de.siegmar.fastcsv.writer.QuoteStrategies
12+
import java.nio.file.Files
13+
import kotlin.test.Test
14+
import kotlin.test.assertEquals
15+
import kotlin.test.assertFailsWith
16+
17+
/**
18+
* Proves that reading a semicolon-delimited CSV with a hardcoded comma
19+
* field separator (as CSVReader.streamRows does in its parallel chunk path)
20+
* crashes with CsvParseException.
21+
*
22+
* Bug: CSVReader.streamRows() at lines 190, 202, 219 hardcodes ','
23+
* when splitting and re-reading chunk files, because fieldSeparator
24+
* is a bare constructor param (not a val) and is inaccessible in methods.
25+
*/
26+
class CSVFieldSeparatorTest {
27+
companion object {
28+
private val HEADER = listOf("qualifiedName", "name", "typeName", "description")
29+
private val ROW1 = listOf("default/schema/table/col1", "col1", "Column", "First column")
30+
private val ROW2 = listOf("default/schema/table/col2", "col2", "Column", "Second column")
31+
}
32+
33+
private fun writeCSV(separator: Char): java.nio.file.Path {
34+
val path = Files.createTempFile("csv_sep_test_", ".csv")
35+
val writer =
36+
CsvWriter
37+
.builder()
38+
.fieldSeparator(separator)
39+
.quoteCharacter('"')
40+
.quoteStrategy(QuoteStrategies.NON_EMPTY)
41+
.lineDelimiter(LineDelimiter.LF)
42+
.build(path)
43+
writer.writeRecord(HEADER)
44+
writer.writeRecord(ROW1)
45+
writer.writeRecord(ROW2)
46+
writer.close()
47+
return path
48+
}
49+
50+
private fun readCSV(
51+
path: java.nio.file.Path,
52+
separator: Char,
53+
): List<List<String>> {
54+
val reader =
55+
CsvReader
56+
.builder()
57+
.fieldSeparator(separator)
58+
.quoteCharacter('"')
59+
.skipEmptyLines(true)
60+
.extraFieldStrategy(FieldMismatchStrategy.STRICT)
61+
.missingFieldStrategy(FieldMismatchStrategy.STRICT)
62+
.ofCsvRecord(path)
63+
val rows = mutableListOf<List<String>>()
64+
reader.stream().skip(1).forEach { r: CsvRecord ->
65+
rows.add(r.fields.toList())
66+
}
67+
reader.close()
68+
return rows
69+
}
70+
71+
@Test
72+
fun semicolonCSVReadWithCorrectSeparator() {
73+
val path = writeCSV(';')
74+
val rows = readCSV(path, ';')
75+
assertEquals(2, rows.size, "Should read 2 data rows")
76+
assertEquals(4, rows[0].size, "Each row should have 4 fields")
77+
assertEquals("default/schema/table/col1", rows[0][0])
78+
assertEquals("col1", rows[0][1])
79+
assertEquals("Column", rows[0][2])
80+
assertEquals("First column", rows[0][3])
81+
Files.deleteIfExists(path)
82+
}
83+
84+
@Test
85+
fun semicolonCSVReadWithHardcodedCommaCrashes() {
86+
// Simulates what CSVReader.streamRows() does at line 219:
87+
// reads semicolon-delimited chunk files with hardcoded fieldSeparator(',')
88+
val path = writeCSV(';')
89+
assertFailsWith<CsvParseException>(
90+
message = "Reading semicolon CSV with comma separator should throw CsvParseException",
91+
) {
92+
readCSV(path, ',')
93+
}
94+
Files.deleteIfExists(path)
95+
}
96+
97+
@Test
98+
fun commaCSVReadWithCommaSeparatorWorks() {
99+
val path = writeCSV(',')
100+
val rows = readCSV(path, ',')
101+
assertEquals(2, rows.size)
102+
assertEquals(4, rows[0].size)
103+
assertEquals("default/schema/table/col1", rows[0][0])
104+
Files.deleteIfExists(path)
105+
}
106+
}

0 commit comments

Comments
 (0)