From 0659cc4d9db114a0803380fee49a32cf1b78e7df Mon Sep 17 00:00:00 2001 From: Simon Parten Date: Wed, 30 Sep 2026 17:04:36 +0200 Subject: [PATCH 1/4] Fix Option[T] JDBC decoder throwing on SQL NULL `JdbcDecoder[Option[T]]` read the column via the strict `JdbcDecoder[T]` and only then asked `rs.wasNull()`. For every reference type that is too late: the strict decoders for String, BigDecimal, Array[Byte] and Instant inspect `wasNull` themselves and throw, so the wrapper inherited the throw and a nullable VARCHAR mapped to `Option[String]` failed on its first NULL row with "Use Option[String] for nullable columns" - advice the caller had already taken. LocalDate, LocalDateTime and UUID had the mirror problem: `getObject` returns null, so a non-Option column decoded to a null rather than reporting anything. Reading the column twice is not an option - JDBC only guarantees one left-to-right read per column, and `JdbcRowDecoder` relies on that. So the null policy moves onto the typeclass instead: `decode` stays strict and `decodeOption` is the same single read with the opposite answer for NULL. The built-ins are now all built by one `strict` combinator that reads once, checks `wasNull`, and only then converts - which is also what stops `BigDecimal(_)` and `Timestamp#toInstant` NPE-ing on the sentinel a NULL read returns. `decodeOption` has a default, so a user-supplied decoder keeps working and still gets `Option` support; it needs overriding only when `decode` itself consults `wasNull`. Primitives are strict now too. `getInt` hands back 0 for a NULL, so a non-Option Int column silently decoded NULLs to zero - a summed column quietly short by however many rows were NULL. It now says which column and what to write instead, consistent with what the String decoder already claimed to do. Co-Authored-By: Claude Opus 5 (1M context) --- db/jvm/src/JdbcDecoder.scala | 154 +++++++++++++++++------------------ db/test/src/DbSuite.scala | 85 +++++++++++++++++++ 2 files changed, 159 insertions(+), 80 deletions(-) diff --git a/db/jvm/src/JdbcDecoder.scala b/db/jvm/src/JdbcDecoder.scala index 29f0fb293..08c9d9913 100644 --- a/db/jvm/src/JdbcDecoder.scala +++ b/db/jvm/src/JdbcDecoder.scala @@ -9,9 +9,30 @@ import java.util.UUID /** Typeclass for reading a single column from a [[java.sql.ResultSet]] by 1-based index. * * Instances are provided for all types supported by the [[Flavour]] type mapping. Custom types can be supported by providing a given instance. + * + * ===Nullability=== + * A column is read from the `ResultSet` exactly once, because JDBC only guarantees that each column of a row is read once, in left-to-right order. That is why nullability is a + * second method on this typeclass rather than a wrapper around `decode`: the wrapper would have to either read the column twice, or let `decode` throw before it could ask whether + * the value was NULL. + * + * [[decode]] is strict - a SQL NULL is a schema disagreement and is reported as one. [[decodeOption]] is the same read with the opposite answer for NULL, and is what the + * `JdbcDecoder[Option[T]]` instance calls. */ trait JdbcDecoder[T]: + /** Read the column, failing if it holds SQL NULL. */ def decode(rs: ResultSet, index: Int): T + + /** Read the column, returning `None` if it holds SQL NULL. + * + * The default is correct for any `decode` that tolerates a NULL read without throwing, which is the case for a decoder built on a JDBC getter that returns a sentinel (`0`, + * `false`, `null`). Override it whenever `decode` inspects `rs.wasNull()` itself, or dereferences the value it read - otherwise `Option[T]` inherits the very failure the + * `Option` was meant to express. + */ + def decodeOption(rs: ResultSet, index: Int): Option[T] = + val v = decode(rs, index) + if rs.wasNull() then None else Some(v) + end if + end decodeOption end JdbcDecoder object JdbcDecoder: @@ -21,99 +42,72 @@ object JdbcDecoder: try rs.getMetaData.getColumnLabel(index) catch case _: Exception => index.toString - given JdbcDecoder[Int] with - def decode(rs: ResultSet, index: Int): Int = rs.getInt(index) - end given + private def nullError(rs: ResultSet, index: Int, typeName: String): java.sql.SQLDataException = + new java.sql.SQLDataException( + s"Column \"${columnLabel(rs, index)}\" (index $index) is NULL but was mapped to a non-nullable $typeName. Use Option[$typeName] for nullable columns." + ) - given JdbcDecoder[Long] with - def decode(rs: ResultSet, index: Int): Long = rs.getLong(index) - end given + /** Builds a decoder that reads the column exactly once and then decides what NULL means. + * + * `read` is the raw JDBC getter; `convert` runs only after the NULL check, so a converter may dereference its argument freely - `BigDecimal(_)` and `Timestamp#toInstant` would + * both NPE if they ran on the sentinel a NULL read returns. + * + * @param typeName + * How the type is spelled in the error message, so it reads as the Scala type the user actually wrote + */ + private def strict[R, T](typeName: String)(read: (ResultSet, Int) => R)(convert: R => T): JdbcDecoder[T] = + new JdbcDecoder[T]: + def decode(rs: ResultSet, index: Int): T = + val raw = read(rs, index) + if rs.wasNull() then throw nullError(rs, index, typeName) + end if + convert(raw) + end decode - given JdbcDecoder[Double] with - def decode(rs: ResultSet, index: Int): Double = rs.getDouble(index) - end given + override def decodeOption(rs: ResultSet, index: Int): Option[T] = + val raw = read(rs, index) + if rs.wasNull() then None else Some(convert(raw)) + end if + end decodeOption - given JdbcDecoder[Float] with - def decode(rs: ResultSet, index: Int): Float = rs.getFloat(index) - end given + private def strict[T](typeName: String)(read: (ResultSet, Int) => T): JdbcDecoder[T] = + strict[T, T](typeName)(read)(identity) - given JdbcDecoder[Boolean] with - def decode(rs: ResultSet, index: Int): Boolean = rs.getBoolean(index) - end given + given JdbcDecoder[Int] = strict("Int")(_.getInt(_)) - given JdbcDecoder[String] with - def decode(rs: ResultSet, index: Int): String = - val v = rs.getString(index) - // If the DB returns NULL for a column typed as String (not Option[String]), - // that means the schema is inconsistent. Throw rather than silently returning "". - // For nullable columns, use Option[String] (which uses the JdbcDecoder[Option[T]] instance). - if rs.wasNull() then - throw new java.sql.SQLDataException( - s"Column \"${columnLabel(rs, index)}\" (index $index) is NULL but was mapped to a non-nullable String. Use Option[String] for nullable columns." - ) - end if - v - end decode - end given + given JdbcDecoder[Long] = strict("Long")(_.getLong(_)) - given JdbcDecoder[BigDecimal] with - def decode(rs: ResultSet, index: Int): BigDecimal = - val v = rs.getBigDecimal(index) - if rs.wasNull() then - throw new java.sql.SQLDataException( - s"Column \"${columnLabel(rs, index)}\" (index $index) is NULL but was mapped to a non-nullable BigDecimal. Use Option[BigDecimal] for nullable columns." - ) - end if - BigDecimal(v) - end decode - end given + given JdbcDecoder[Double] = strict("Double")(_.getDouble(_)) - given JdbcDecoder[Array[Byte]] with - def decode(rs: ResultSet, index: Int): Array[Byte] = - val v = rs.getBytes(index) - if rs.wasNull() then - throw new java.sql.SQLDataException( - s"Column \"${columnLabel(rs, index)}\" (index $index) is NULL but was mapped to a non-nullable Array[Byte]. Use Option[Array[Byte]] for nullable columns." - ) - end if - v - end decode - end given + given JdbcDecoder[Float] = strict("Float")(_.getFloat(_)) - given JdbcDecoder[LocalDate] with - def decode(rs: ResultSet, index: Int): LocalDate = - rs.getObject(index, classOf[LocalDate]) - end given + given JdbcDecoder[Boolean] = strict("Boolean")(_.getBoolean(_)) - given JdbcDecoder[LocalDateTime] with - def decode(rs: ResultSet, index: Int): LocalDateTime = - rs.getObject(index, classOf[LocalDateTime]) - end given + given JdbcDecoder[String] = strict("String")(_.getString(_)) - given JdbcDecoder[Instant] with - def decode(rs: ResultSet, index: Int): Instant = - val ts = rs.getTimestamp(index) - if rs.wasNull() then - throw new java.sql.SQLDataException( - s"Column \"${columnLabel(rs, index)}\" (index $index) is NULL but was mapped to a non-nullable Instant. Use Option[Instant] for nullable columns." - ) - end if - ts.toInstant - end decode - end given + given JdbcDecoder[BigDecimal] = strict("BigDecimal")(_.getBigDecimal(_))(BigDecimal(_)) - given JdbcDecoder[UUID] with - def decode(rs: ResultSet, index: Int): UUID = - rs.getObject(index, classOf[UUID]) - end given + given JdbcDecoder[Array[Byte]] = strict("Array[Byte]")(_.getBytes(_)) - /** Nullable column: returns `None` when the DB value is SQL NULL. */ + given JdbcDecoder[LocalDate] = strict("LocalDate")(_.getObject(_, classOf[LocalDate])) + + given JdbcDecoder[LocalDateTime] = strict("LocalDateTime")(_.getObject(_, classOf[LocalDateTime])) + + given JdbcDecoder[Instant] = strict("Instant")(_.getTimestamp(_))(_.toInstant) + + given JdbcDecoder[UUID] = strict("UUID")(_.getObject(_, classOf[UUID])) + + /** Nullable column: returns `None` when the DB value is SQL NULL. + * + * Delegates to [[JdbcDecoder.decodeOption]] rather than calling `decode` and testing `rs.wasNull()` afterwards. The latter cannot work: a strict decoder throws on NULL, so the + * test would never be reached, and every reference-typed `Option` column would fail on its first NULL row. + */ given [T](using inner: JdbcDecoder[T]): JdbcDecoder[Option[T]] with - def decode(rs: ResultSet, index: Int): Option[T] = - val v = inner.decode(rs, index) - if rs.wasNull() then None else Some(v) - end if - end decode + def decode(rs: ResultSet, index: Int): Option[T] = inner.decodeOption(rs, index) + + /** `Option[Option[T]]` is not a column shape, but the default would read the column a second time. */ + override def decodeOption(rs: ResultSet, index: Int): Option[Option[T]] = + Some(inner.decodeOption(rs, index)) end given end JdbcDecoder diff --git a/db/test/src/DbSuite.scala b/db/test/src/DbSuite.scala index 81916770d..57906b46d 100644 --- a/db/test/src/DbSuite.scala +++ b/db/test/src/DbSuite.scala @@ -162,6 +162,91 @@ class JdbcDecoderSuite extends H2Fixture: } } + /** A NULL in a column mapped to `Option[T]` must reach the caller as `None`, whatever `T` is. + * + * The reference types are the ones that used to fail here. Their strict decoders inspect `rs.wasNull()` themselves and throw, so an `Option` wrapper built as "decode, then ask + * whether it was null" never got to ask - it inherited the throw, and told the user to do the very thing they had already done. + */ + test("decode Option[T] - None for NULL, for every supported T") { + withConn { conn => + def nullOf[T](sqlType: String)(using d: JdbcDecoder[Option[T]]): Option[T] = + val rs = conn.createStatement().executeQuery(s"SELECT CAST(NULL AS $sqlType)") + rs.next() + d.decode(rs, 1) + end nullOf + + assertEquals(nullOf[Int]("INT"), None) + assertEquals(nullOf[Long]("BIGINT"), None) + assertEquals(nullOf[Double]("DOUBLE"), None) + assertEquals(nullOf[Boolean]("BOOLEAN"), None) + assertEquals(nullOf[String]("VARCHAR(10)"), None) + assertEquals(nullOf[BigDecimal]("DECIMAL(10,3)"), None) + assertEquals(nullOf[Array[Byte]]("VARBINARY(10)"), None) + assertEquals(nullOf[java.time.LocalDate]("DATE"), None) + assertEquals(nullOf[java.time.LocalDateTime]("TIMESTAMP"), None) + assertEquals(nullOf[java.time.Instant]("TIMESTAMP"), None) + assertEquals(nullOf[java.util.UUID]("UUID"), None) + } + } + + test("decode Option[T] - Some for a present value, for every supported T") { + withConn { conn => + def someOf[T](expr: String)(using d: JdbcDecoder[Option[T]]): Option[T] = + val rs = conn.createStatement().executeQuery(s"SELECT $expr") + rs.next() + d.decode(rs, 1) + end someOf + + assertEquals(someOf[String]("'hello'"), Some("hello")) + assertEquals(someOf[BigDecimal]("CAST(123.456 AS DECIMAL(10,3))"), Some(BigDecimal("123.456"))) + assertEquals(someOf[java.time.LocalDate]("DATE '2024-03-01'"), Some(java.time.LocalDate.of(2024, 3, 1))) + assertEquals(someOf[Int]("42"), Some(42)) + // Array[Byte] has no useful equals, so compare the contents + assertEquals(someOf[Array[Byte]]("CAST(X'01ff' AS VARBINARY(10))").map(_.toList), Some(List[Byte](1, -1))) + } + } + + /** The other half of the contract: a NULL in a column mapped to a bare `T` is a schema disagreement, and is reported as one rather than decoded to a sentinel. + * + * This matters most for the primitives, where JDBC hands back `0` / `false` for a NULL and the mistake would otherwise be invisible - a summed column quietly short by however + * many rows were NULL. + */ + test("decode T - a NULL in a non-Option column names the column and the fix") { + withConn { conn => + def strictNull[T](sqlType: String)(using d: JdbcDecoder[T]): Unit = + val rs = conn.createStatement().executeQuery(s"SELECT CAST(NULL AS $sqlType) AS the_column") + rs.next() + val e = intercept[java.sql.SQLDataException](d.decode(rs, 1)) + // H2 upper cases unquoted identifiers, so compare case insensitively + assert(e.getMessage.toLowerCase.contains("the_column"), e.getMessage) + assert(e.getMessage.contains("Use Option["), e.getMessage) + end strictNull + + strictNull[Int]("INT") + strictNull[Long]("BIGINT") + strictNull[Double]("DOUBLE") + strictNull[Boolean]("BOOLEAN") + strictNull[String]("VARCHAR(10)") + strictNull[BigDecimal]("DECIMAL(10,3)") + strictNull[Array[Byte]]("VARBINARY(10)") + strictNull[java.time.LocalDate]("DATE") + strictNull[java.time.Instant]("TIMESTAMP") + strictNull[java.util.UUID]("UUID") + } + } + + /** A whole row of NULLs, decoded positionally, is the shape the bug actually reached users in. */ + test("a row of nullable reference columns decodes to all None") { + withConn { conn => + val rs = conn + .createStatement() + .executeQuery("SELECT CAST(NULL AS VARCHAR(10)), CAST(NULL AS DECIMAL(10,3)), CAST(NULL AS DATE), CAST(NULL AS BIGINT)") + rs.next() + type Row = (Option[String], Option[BigDecimal], Option[java.time.LocalDate], Option[Long]) + assertEquals(summon[JdbcRowDecoder[Row]].decodeRow(rs), (None, None, None, None)) + } + } + end JdbcDecoderSuite class JdbcRowDecoderSuite extends H2Fixture: From f499541b8c32d49d958bc14e807fc26b640c8291 Mon Sep 17 00:00:00 2001 From: Simon Parten Date: Wed, 30 Sep 2026 17:07:02 +0200 Subject: [PATCH 2/4] Fix ExcelIterator running off the end on a blank row `hasNext` compared a counter against the range's last row, while `sheetIterator` skips rows POI reports as absent. An entirely blank row inside the range - a visual separator between groups of data, which is ordinary in real spreadsheets - therefore made `hasNext` promise more rows than the iterator had, and `next()` fell off the end with a bare `java.util.NoSuchElementException: next on empty iterator`. On a sheet spanning A1:B6 with one absent row, four rows came back and then it threw. Pinned ranges are the documented recommendation for Excel, so this sat on the recommended path. `hasNext` now asks the row iterator. The range still bounds iteration, because `sheetIterator` is built from `firstRow + 1 to lastRow`, and skipping blank rows keeps reading consistent with compile time inference, which walks the range the same way. `currentRowIndex` is now read off the row rather than counted, so the row number in a decode error names the row Excel shows; counting drifted as soon as a blank row was skipped. The parsed range is memoised too - it was being re-parsed from its string form on every `hasNext` and every row. Co-Authored-By: Claude Opus 5 (1M context) --- scautable/src-jvm/ExcelIterator.scala | 62 +++++++++++++------------- scautable/test/src-jvm/testExcel.scala | 42 +++++++++++++++++ 2 files changed, 73 insertions(+), 31 deletions(-) diff --git a/scautable/src-jvm/ExcelIterator.scala b/scautable/src-jvm/ExcelIterator.scala index 1ff678fca..b434d4884 100644 --- a/scautable/src-jvm/ExcelIterator.scala +++ b/scautable/src-jvm/ExcelIterator.scala @@ -40,6 +40,9 @@ class ExcelIterator[K <: Tuple, V <: Tuple](filePath: String, sheetName: String, (cellRange.getFirstRow, cellRange.getLastRow, cellRange.getFirstColumn, cellRange.getLastColumn) end parseRange + /** The range's corners, parsed once. `CellRangeAddress.valueOf` is string parsing, and this sits on the per-row path. */ + private lazy val parsedRange: Option[(Int, Int, Int, Int)] = colRange.filter(_.nonEmpty).map(parseRange) + /** Validates that headers are unique (no duplicates) */ private def validateUniqueHeaders(headers: List[String]): Unit = @@ -59,32 +62,28 @@ class ExcelIterator[K <: Tuple, V <: Tuple](filePath: String, sheetName: String, ) val sheet = workbook.getSheet(sheetName) // Create an iterator that gives us rows by index for the specified range - colRange match - case Some(range) if range.nonEmpty => - val (firstRow, lastRow, _, _) = parseRange(range) + parsedRange match + case Some((firstRow, lastRow, _, _)) => val dataStartRow = firstRow + 1 val dataRowIndices = (dataStartRow to lastRow).toIterator dataRowIndices.map(rowIndex => sheet.getRow(rowIndex)).filter(_ != null) - case _ => + case None => sheet.iterator().asScala // For no range, use default iterator end match end sheetIterator - // Track current row number for error reporting - starts where data begins - private var currentRowIndex: Int = colRange match - case None => 0 - case Some(range) if range.nonEmpty => - val (firstRow, _, _, _) = parseRange(range) - firstRow + 1 // Skip the header row - data starts at firstRow + 1 - case _ => 0 + /** The spreadsheet row number (1 based, as Excel shows it) of the row last returned by `next()`, for error reporting. + * + * Taken from the row itself rather than counted, because the iterator skips rows POI reports as absent - a counter would drift from the sheet the moment it met one, and name + * the wrong row in an error. + */ + private var currentRowIndex: Int = 0 // Extract headers from the first row or specified range private val headers: List[String] = - colRange match - case Some(range) if range.nonEmpty => - extractHeadersFromRange(range) - case _ => - extractHeadersFromFirstRow() + parsedRange match + case Some(_) => extractHeadersFromRange() + case None => extractHeadersFromFirstRow() private lazy val numCellsPerRow = headers.size @@ -93,8 +92,8 @@ class ExcelIterator[K <: Tuple, V <: Tuple](filePath: String, sheetName: String, /** Extract headers from a specified cell range This accesses the header row directly by index */ - private def extractHeadersFromRange(range: String): List[String] = - val (firstRow, _, firstCol, lastCol) = parseRange(range) + private def extractHeadersFromRange(): List[String] = + val (firstRow, _, firstCol, lastCol) = parsedRange.get val workbook = ExcelWorkbookCache .getOrCreate(filePath) .getOrElse( @@ -118,14 +117,13 @@ class ExcelIterator[K <: Tuple, V <: Tuple](filePath: String, sheetName: String, /** Extract cell values from a row based on the column range */ private def extractCellValues(row: org.apache.poi.ss.usermodel.Row): List[String] = - colRange match - case Some(range) if range.nonEmpty => - val (_, _, firstCol, lastCol) = parseRange(range) + parsedRange match + case Some((_, _, firstCol, lastCol)) => val cells = for i <- firstCol.to(lastCol) yield row.getCell(i, Row.MissingCellPolicy.CREATE_NULL_AS_BLANK).toString cells.toList - case _ => + case None => row.cellIterator().asScala.toList.map(_.toString) end extractCellValues @@ -134,6 +132,7 @@ class ExcelIterator[K <: Tuple, V <: Tuple](filePath: String, sheetName: String, end if val row = sheetIterator.next() + currentRowIndex = row.getRowNum + 1 val cellValues = extractCellValues(row) // Validate row has expected number of cells @@ -152,17 +151,18 @@ class ExcelIterator[K <: Tuple, V <: Tuple](filePath: String, sheetName: String, ) ) - currentRowIndex += 1 NamedTuple.build[K]()(decodedTuple) end next - override def hasNext: Boolean = - colRange match - case Some(range) if range.nonEmpty => - val (_, lastRow, _, _) = parseRange(range) - currentRowIndex <= lastRow - case _ => - sheetIterator.hasNext - end hasNext + /** Whether another row is actually available. + * + * Asks the row iterator, rather than comparing a counter against the range's last row. The two are not the same: `sheetIterator` drops rows POI reports as absent, which is what + * an entirely blank row inside the range is - a visual separator between groups, say. Counting row numbers therefore promised more rows than existed, and `next()` fell off the + * end of the underlying iterator with a bare `NoSuchElementException`. + * + * The range still bounds the iteration, because `sheetIterator` is built from `firstRow + 1 to lastRow`. Skipping blank rows also keeps reading consistent with compile time + * type inference, which walks the range the same way. + */ + override def hasNext: Boolean = sheetIterator.hasNext end ExcelIterator diff --git a/scautable/test/src-jvm/testExcel.scala b/scautable/test/src-jvm/testExcel.scala index 6b414f48d..aa1642475 100644 --- a/scautable/test/src-jvm/testExcel.scala +++ b/scautable/test/src-jvm/testExcel.scala @@ -166,6 +166,48 @@ class ExcelSuite extends munit.FunSuite: assertEquals(one.column["Column 2"].toList(0), "Row 1, Col 2") } + /** A row POI reports as absent - one that was never given a cell, which is what a blank separator row between groups of data is - used to desynchronise reading from counting. + * + * `sheetIterator` skips such rows; `hasNext` used to compare a counter against the range's last row and so promised one result per row *number*. On a range spanning a blank + * row, `next()` therefore ran off the end of the underlying iterator and threw a bare `NoSuchElementException: next on empty iterator`. + */ + test("a blank row inside a pinned range does not derail the iterator") { + val dir = os.temp.dir() + val path = (dir / "gappy.xlsx").toString + val wb = new org.apache.poi.xssf.usermodel.XSSFWorkbook() + val sheet = wb.createSheet("Sheet1") + def put(rowIdx: Int, a: String, b: String): Unit = + val row = sheet.createRow(rowIdx) + row.createCell(0).setCellValue(a) + row.createCell(1).setCellValue(b) + end put + put(0, "ca", "cb") // header + put(1, "1", "2") + put(2, "3", "4") + // row index 3 is deliberately never created, so POI reports it as absent + put(4, "5", "6") + put(5, "7", "8") + val out = new java.io.FileOutputStream(path) + wb.write(out) + out.close() + wb.close() + + val itr = new ExcelIterator[("ca", "cb"), (String, String)](path, "Sheet1", Some("A1:B6")) + val rows = itr.toList + assertEquals(rows.map(r => (r.ca, r.cb)), List(("1", "2"), ("3", "4"), ("5", "6"), ("7", "8"))) + + // and the iterator agrees with itself about when it is done + val second = new ExcelIterator[("ca", "cb"), (String, String)](path, "Sheet1", Some("A1:B6")) + var count = 0 + while second.hasNext do + second.next() + count += 1 + end while + assertEquals(count, 4) + assert(!second.hasNext) + intercept[NoSuchElementException](second.next()) + } + test("excel provider with FromTuple TypeInferrer enforces specific types") { // Force specific types using FromTuple - all columns are actually strings in SimpleTable.xlsx def csv = Excel.resource("SimpleTable.xlsx", "Sheet1", "", TypeInferrer.FromTuple[(String, String, String)]()) From 22028296616492a94548d23b8d4d54551b97b696 Mon Sep 17 00:00:00 2001 From: Simon Parten Date: Thu, 1 Oct 2026 09:17:33 +0200 Subject: [PATCH 3/4] Give the CSV and JSON iterators the handle they read from `CSV.resource(...)` opened a `scala.io.Source`, kept its line iterator and dropped the `Source` itself, so nothing ever closed it; `JsonTable` did the same with its `InputStream`. The handles came back only when the JVM noticed the objects were unreachable, which is driven by heap pressure rather than by reading - measured here, 20k reads of a small CSV stacked up a high water mark of ~5,200 open descriptors before any were returned, which is already past a 1024 `ulimit -n`. On Windows the file stays locked for just as long. `parquet` and `db` already got this right, so the two oldest readers were the odd ones out. Both iterators now own an optional `AutoCloseable` and close it when the rows run out, so the ordinary shapes - `.toSeq`, `foreach`, a forced `LazyList` - need nothing. Both are `AutoCloseable` too, for stopping early: Using(CSV.absolutePath("big.csv"))(_.take(10).toList) The same measurement after this change stays flat at the baseline for a drained read and for `Using`. Abandoning a partially read iterator without closing it still accumulates, and cannot not: `.take(10)` returns a *new* iterator with no way to tell the original it is finished. That is what `close` is for, and the docs now say so. `next()` refuses to read past a close rather than returning whatever the reader had buffered - a test caught that, not inspection. Also closed three handles that were only released on the happy path: the column and dense-array builders dropped theirs whenever `parseLine` met a malformed line, `fromTyped` stranded one on each of its three header validation failures, and the macro's own handle on the CSV was never closed at all - one descriptor per expansion, for the lifetime of a build daemon. ExcelWorkbookCache had a related bug of its own: when a cached workbook failed its liveness check the replacement was never put back in the cache, so every later call opened a fresh workbook - each holding a file handle - and cached none of them. `compute` now does the lookup and the insert in one step, which also closes the check-then-act race where two threads each opened a workbook and the loser's was leaked, despite the method's documented thread safety. Co-Authored-By: Claude Opus 5 (1M context) --- scautable/src-jvm/ExcelWorkbookCache.scala | 43 +- scautable/src-jvm/JsonIterator.scala | 37 +- scautable/src-jvm/JsonTable.scala | 62 +-- scautable/src/csv.scala | 380 +++++++++--------- scautable/src/csvIterator.scala | 51 ++- .../test/src/IteratorResourceSuite.scala | 78 ++++ site/docs/csv.md | 24 ++ 7 files changed, 445 insertions(+), 230 deletions(-) create mode 100644 scautable/test/src/IteratorResourceSuite.scala diff --git a/scautable/src-jvm/ExcelWorkbookCache.scala b/scautable/src-jvm/ExcelWorkbookCache.scala index 139e8daa1..aa0198085 100644 --- a/scautable/src-jvm/ExcelWorkbookCache.scala +++ b/scautable/src-jvm/ExcelWorkbookCache.scala @@ -35,29 +35,38 @@ object ExcelWorkbookCache: // Try to get existing workbook from cache val cachedRef = cache.get(normalizedPath) - val existingWorkbook = Option(cachedRef).flatMap(ref => Option(ref.get())) + val existingWorkbook = Option(cachedRef).flatMap(ref => Option(ref.get())).filter(isUsable) existingWorkbook match - case Some(workbook) => - // Validate that the workbook is still usable (not closed) - try - // Simple validation - try to access the number of sheets - workbook.getNumberOfSheets - workbook - catch - case _: Exception => - // Workbook is no longer valid, remove from cache and create new one - cache.remove(normalizedPath) - WorkbookFactory.create(new File(normalizedPath), null, true) - case None => - // No cached workbook or it was garbage collected - val workbook = WorkbookFactory.create(new File(normalizedPath), null, true) - cache.put(normalizedPath, new WeakReference(workbook)) - workbook + case Some(workbook) => workbook + case None => + // Either nothing cached, or what was cached has been garbage collected or closed underneath us. + // + // `compute` rather than get-then-put: the whole point of the cache is that one file yields one workbook, and a plain check-then-act lets two threads each open one, with + // whichever loses the race left open and unreferenced. Each open holds an OS file handle, so losing that race leaks one. + cache + .compute( + normalizedPath, + (_, existing) => + val live = Option(existing).flatMap(ref => Option(ref.get())).filter(isUsable) + live match + case Some(_) => existing // another thread got there first; keep its workbook + case None => new WeakReference(WorkbookFactory.create(new File(normalizedPath), null, true)) + end match + ) + .get() end match } end getOrCreate + /** Whether a cached workbook can still be read from. A workbook closed behind the cache's back throws from any access, and has to be replaced rather than handed out. */ + private def isUsable(workbook: Workbook): Boolean = + try + workbook.getNumberOfSheets + true + catch case _: Exception => false + end isUsable + /** Explicitly close and remove a workbook from the cache. * * This method should be called when you know a workbook will no longer be needed to free up resources immediately rather than waiting for garbage collection. diff --git a/scautable/src-jvm/JsonIterator.scala b/scautable/src-jvm/JsonIterator.scala index b4badf7aa..35bc81ef9 100644 --- a/scautable/src-jvm/JsonIterator.scala +++ b/scautable/src-jvm/JsonIterator.scala @@ -27,17 +27,48 @@ import io.github.quafadas.scautable.json.StreamingJsonParser.* */ class JsonIterator[K <: Tuple, V <: Tuple] @publicInBinary private[json] ( private val objects: Iterator[JsonObject], - val headers: Seq[String] + val headers: Seq[String], + private val source: Option[AutoCloseable] = None )(using decoder: RowDecoder[V]) - extends Iterator[NamedTuple[K, V]]: + extends Iterator[NamedTuple[K, V]] + with AutoCloseable: type COLUMNS = K type Col[N <: Int] = Tuple.Elem[K, N] - override def hasNext: Boolean = objects.hasNext + private var closed = false + + /** Release the stream behind this iterator. + * + * Idempotent, and safe to call at any point - a closed iterator simply reports no more rows. Draining the iterator calls this for you, so `.toSeq` and friends need nothing. + * Reach for it when you stop reading early, or let `scala.util.Using` do it. + */ + override def close(): Unit = + if !closed then + closed = true + source.foreach { s => + try s.close() + catch case _: Exception => () // a handle we cannot release is not worth failing a read over + } + end if + end close + + /** Closes the underlying stream as soon as the objects run out, so that reading a whole file needs no cleanup. */ + override def hasNext: Boolean = + if closed then false + else + val more = objects.hasNext + if !more then close() + end if + more + end if + end hasNext override def next(): NamedTuple[K, V] = + // Past a close the underlying stream is gone, so reading on would hand back whatever happened to be buffered, or fail deep inside the parser. + if closed then throw new NoSuchElementException("This JsonIterator has been closed, so there are no more rows to read.") + end if val obj = objects.next() // Extract values in header order, converting JsonValue to String val values = headers.map { header => diff --git a/scautable/src-jvm/JsonTable.scala b/scautable/src-jvm/JsonTable.scala index 7d93dbd89..abd8a9f40 100644 --- a/scautable/src-jvm/JsonTable.scala +++ b/scautable/src-jvm/JsonTable.scala @@ -285,14 +285,15 @@ object JsonTable: end if val inputStream = resourceUrl.openStream() val objects = StreamingJsonParser.parseArrayStream(inputStream) - new JsonIterator[Hdrs, Data](objects, ${ Expr.ofSeq(headers.map(Expr(_))) }.toSeq) + // Handed to the iterator rather than dropped, so that draining it - or closing it early - releases the stream. + new JsonIterator[Hdrs, Data](objects, ${ Expr.ofSeq(headers.map(Expr(_))) }.toSeq, Some(inputStream)) } else '{ val path = $pathExpr val inputStream = new java.io.FileInputStream(path) val objects = StreamingJsonParser.parseArrayStream(inputStream) - new JsonIterator[Hdrs, Data](objects, ${ Expr.ofSeq(headers.map(Expr(_))) }.toSeq) + new JsonIterator[Hdrs, Data](objects, ${ Expr.ofSeq(headers.map(Expr(_))) }.toSeq, Some(inputStream)) } end constructIterator @@ -364,30 +365,37 @@ object JsonTable: inline def fromTyped[K <: Tuple, V <: Tuple](using decoder: RowDecoder[V]): os.Path => JsonIterator[K, V] = (path: os.Path) => val inputStream = new java.io.FileInputStream(path.toIO) - val objects = StreamingJsonParser.parseArrayStream(inputStream) - - // Peek at first object to validate headers match expected - val bufferedObjects = objects.buffered - if !bufferedObjects.hasNext then throw new IllegalStateException(s"JSON file at ${path.toString} contains no objects") - end if - - val firstObj = bufferedObjects.head - val actualHeaders = firstObj.fields.keys.toSeq - val expectedHeaders = scala.compiletime.constValueTuple[K].toArray.toSeq.asInstanceOf[Seq[String]] - - // Validate headers match (order doesn't need to match for JSON, but all expected fields must be present) - val missingHeaders = expectedHeaders.filterNot(actualHeaders.contains) - if missingHeaders.nonEmpty then - throw new IllegalStateException( - s"JSON object missing expected fields. Expected: $expectedHeaders, Got: $actualHeaders. Missing: $missingHeaders" - ) - end if - - val sizeOfV = scala.compiletime.constValue[Tuple.Size[V]] - if expectedHeaders.length != sizeOfV then - throw new IllegalStateException(s"Number of expected headers (${expectedHeaders.length}) does not match number (${sizeOfV}) of types provided for decoding.") - end if - - new JsonIterator[K, V](bufferedObjects, expectedHeaders) + // Every check below can throw, and each one used to strand the stream it had just opened. + try + val objects = StreamingJsonParser.parseArrayStream(inputStream) + + // Peek at first object to validate headers match expected + val bufferedObjects = objects.buffered + if !bufferedObjects.hasNext then throw new IllegalStateException(s"JSON file at ${path.toString} contains no objects") + end if + + val firstObj = bufferedObjects.head + val actualHeaders = firstObj.fields.keys.toSeq + val expectedHeaders = scala.compiletime.constValueTuple[K].toArray.toSeq.asInstanceOf[Seq[String]] + + // Validate headers match (order doesn't need to match for JSON, but all expected fields must be present) + val missingHeaders = expectedHeaders.filterNot(actualHeaders.contains) + if missingHeaders.nonEmpty then + throw new IllegalStateException( + s"JSON object missing expected fields. Expected: $expectedHeaders, Got: $actualHeaders. Missing: $missingHeaders" + ) + end if + + val sizeOfV = scala.compiletime.constValue[Tuple.Size[V]] + if expectedHeaders.length != sizeOfV then + throw new IllegalStateException(s"Number of expected headers (${expectedHeaders.length}) does not match number (${sizeOfV}) of types provided for decoding.") + end if + + new JsonIterator[K, V](bufferedObjects, expectedHeaders, Some(inputStream)) + catch + case e: Throwable => + inputStream.close() + throw e + end try end JsonTable diff --git a/scautable/src/csv.scala b/scautable/src/csv.scala index 40f05c024..fa3a2989c 100644 --- a/scautable/src/csv.scala +++ b/scautable/src/csv.scala @@ -437,9 +437,10 @@ object CSV: val resourceNameExpr = Expr(pathChain.resourceName) val useFallbackExpr = Expr(pathChain.useFallback) '{ - val lines = CSV.openSourceWithFallback($absolutePathExpr, $rootRelativeExpr, $resourceNameExpr, $useFallbackExpr).getLines() - val (headers, iterator) = lines.headers($csvHeadersExpr, $delimiterExpr) - new CsvIterator[Hdrs, Data](iterator, headers, $delimiterExpr) + val source = CSV.openSourceWithFallback($absolutePathExpr, $rootRelativeExpr, $resourceNameExpr, $useFallbackExpr) + val (headers, iterator) = source.getLines().headers($csvHeadersExpr, $delimiterExpr) + // Handed to the iterator rather than dropped, so that draining it - or closing it early - releases the file handle. + new CsvIterator[Hdrs, Data](iterator, headers, $delimiterExpr, Some(source)) } end constructRowIterator @@ -450,22 +451,24 @@ object CSV: val useFallbackExpr = Expr(pathChain.useFallback) '{ val source = CSV.openSourceWithFallback($absolutePathExpr, $rootRelativeExpr, $resourceNameExpr, $useFallbackExpr) - val lines = source.getLines() - val (headers, iterator) = lines.headers($csvHeadersExpr, $delimiterExpr) - - val numCols = headers.length - val buffers = Array.fill(numCols)(scala.collection.mutable.ArrayBuffer[String]()) - - iterator.foreach { line => - val parsed = CSVParser.parseLine(line, $delimiterExpr) - var i = 0 - while i < parsed.length && i < numCols do - buffers(i) += parsed(i) - i += 1 - end while - } - - source.close() + // `finally`, because a malformed line makes `parseLine` throw, and the handle would otherwise be stranded. + val buffers = + try + val (headers, iterator) = source.getLines().headers($csvHeadersExpr, $delimiterExpr) + val numCols = headers.length + val bufs = Array.fill(numCols)(scala.collection.mutable.ArrayBuffer[String]()) + iterator.foreach { line => + val parsed = CSVParser.parseLine(line, $delimiterExpr) + var i = 0 + while i < parsed.length && i < numCols do + bufs(i) += parsed(i) + i += 1 + end while + } + bufs + finally source.close() + end try + end buffers val typedColumns = ColumnsDecoder.decodeAllColumns[ArrayData](buffers) NamedTuple.build[Hdrs & Tuple]()(typedColumns) @@ -485,22 +488,24 @@ object CSV: } val buffersExpr = '{ val source = CSV.openSourceWithFallback($absolutePathExpr, $rootRelativeExpr, $resourceNameExpr, $useFallbackExpr) - val lines = source.getLines() - val (headers, iterator) = lines.headers($csvHeadersExpr, $delimiterExpr) - - val numCols = headers.length - val buffers = Array.fill(numCols)(scala.collection.mutable.ArrayBuffer[String]()) - - iterator.foreach { line => - val parsed = CSVParser.parseLine(line, $delimiterExpr) - var i = 0 - while i < parsed.length && i < numCols do - buffers(i) += parsed(i) - i += 1 - end while - } - - source.close() + // `finally`, because a malformed line makes `parseLine` throw, and the handle would otherwise be stranded. + val buffers = + try + val (headers, iterator) = source.getLines().headers($csvHeadersExpr, $delimiterExpr) + val numCols = headers.length + val bufs = Array.fill(numCols)(scala.collection.mutable.ArrayBuffer[String]()) + iterator.foreach { line => + val parsed = CSVParser.parseLine(line, $delimiterExpr) + var i = 0 + while i < parsed.length && i < numCols do + bufs(i) += parsed(i) + i += 1 + end while + } + bufs + finally source.close() + end try + end buffers buffers } CSV.buildDenseArrayColMajor[T](buffersExpr, decoderExpr, ct) @@ -519,137 +524,145 @@ object CSV: } val buffersExpr = '{ val source = CSV.openSourceWithFallback($absolutePathExpr, $rootRelativeExpr, $resourceNameExpr, $useFallbackExpr) - val lines = source.getLines() - val (headers, iterator) = lines.headers($csvHeadersExpr, $delimiterExpr) - - val numCols = headers.length - val buffers = Array.fill(numCols)(scala.collection.mutable.ArrayBuffer[String]()) - - iterator.foreach { line => - val parsed = CSVParser.parseLine(line, $delimiterExpr) - var i = 0 - while i < parsed.length && i < numCols do - buffers(i) += parsed(i) - i += 1 - end while - } - - source.close() + // `finally`, because a malformed line makes `parseLine` throw, and the handle would otherwise be stranded. + val buffers = + try + val (headers, iterator) = source.getLines().headers($csvHeadersExpr, $delimiterExpr) + val numCols = headers.length + val bufs = Array.fill(numCols)(scala.collection.mutable.ArrayBuffer[String]()) + iterator.foreach { line => + val parsed = CSVParser.parseLine(line, $delimiterExpr) + var i = 0 + while i < parsed.length && i < numCols do + bufs(i) += parsed(i) + i += 1 + end while + } + bufs + finally source.close() + end try + end buffers buffers } CSV.buildDenseArrayRowMajor[T](buffersExpr, decoderExpr, ct) end constructDenseArrayRowMajor - // Handle dense array modes first - denseColMajorType match - case Some(elemType) => - elemType.asType match - case '[t] => - given Expr[scala.reflect.ClassTag[t]] = Expr.summon[scala.reflect.ClassTag[t]].getOrElse { - report.throwError(s"ClassTag not found for type ${elemType.show}") - } - constructDenseArrayColMajor[t] - case None => - denseRowMajorType match - case Some(elemType) => - elemType.asType match - case '[t] => - given Expr[scala.reflect.ClassTag[t]] = Expr.summon[scala.reflect.ClassTag[t]].getOrElse { - report.throwError(s"ClassTag not found for type ${elemType.show}") - } - constructDenseArrayRowMajor[t] - case None => - // Handle rows or columns mode - if !isColumnMode then - headerTupleExpr match - case '{ $tup: hdrs } => - typeInferrerExpr match - - case '{ TypeInferrer.FromTuple[t]() } => - constructRowIterator[hdrs & Tuple, t & Tuple] - - case '{ TypeInferrer.StringType } => - constructRowIterator[hdrs & Tuple, StringyTuple[hdrs & Tuple] & Tuple] - - case '{ TypeInferrer.FirstRow } => - val inferredTypeRepr = InferrerOps.inferrer(iter, true, delimiter = delimiter) - inferredTypeRepr.asType match - case '[v] => - constructRowIterator[hdrs & Tuple, v & Tuple] - end match - - case '{ TypeInferrer.FromAllRows } => - val inferredTypeRepr = InferrerOps.inferrer(iter, false, Int.MaxValue, delimiter) - inferredTypeRepr.asType match - case '[v] => constructRowIterator[hdrs & Tuple, v & Tuple] - end match - - case '{ TypeInferrer.FirstN(${ Expr(n) }) } => - val inferredTypeRepr = InferrerOps.inferrer(iter, true, n, delimiter) - inferredTypeRepr.asType match - case '[v] => constructRowIterator[hdrs & Tuple, v & Tuple] - end match - - case '{ TypeInferrer.FirstN(${ Expr(n) }, ${ Expr(preferIntToBoolean) }) } => - val inferredTypeRepr = InferrerOps.inferrer(iter, preferIntToBoolean, n, delimiter) - inferredTypeRepr.asType match - case '[v] => constructRowIterator[hdrs & Tuple, v & Tuple] - end match - - case _ => - report.throwError("Could not infer literal header tuple.") - end match - else // isColumnMode - headerTupleExpr match - case '{ $tup: hdrs } => - typeInferrerExpr match - - case '{ TypeInferrer.FromTuple[t]() } => - val arrayTypeRepr = toArrayTupleType(TypeRepr.of[t]) - arrayTypeRepr.asType match - case '[arrTup] => constructColumnArrays[hdrs & Tuple, arrTup & Tuple] - end match - - case '{ TypeInferrer.StringType } => - val stringyType = TypeRepr.of[StringyTuple[hdrs & Tuple]] - val arrayTypeRepr = toArrayTupleType(stringyType) - arrayTypeRepr.asType match - case '[arrTup] => constructColumnArrays[hdrs & Tuple, arrTup & Tuple] - end match - - case '{ TypeInferrer.FirstRow } => - val inferredTypeRepr = InferrerOps.inferrer(iter, true, delimiter = delimiter) - val arrayTypeRepr = toArrayTupleType(inferredTypeRepr) - arrayTypeRepr.asType match - case '[arrTup] => constructColumnArrays[hdrs & Tuple, arrTup & Tuple] - end match - - case '{ TypeInferrer.FromAllRows } => - val inferredTypeRepr = InferrerOps.inferrer(iter, false, Int.MaxValue, delimiter) - val arrayTypeRepr = toArrayTupleType(inferredTypeRepr) - arrayTypeRepr.asType match - case '[arrTup] => constructColumnArrays[hdrs & Tuple, arrTup & Tuple] - end match - - case '{ TypeInferrer.FirstN(${ Expr(n) }) } => - val inferredTypeRepr = InferrerOps.inferrer(iter, true, n, delimiter) - val arrayTypeRepr = toArrayTupleType(inferredTypeRepr) - arrayTypeRepr.asType match - case '[arrTup] => constructColumnArrays[hdrs & Tuple, arrTup & Tuple] - end match - - case '{ TypeInferrer.FirstN(${ Expr(n) }, ${ Expr(preferIntToBoolean) }) } => - val inferredTypeRepr = InferrerOps.inferrer(iter, preferIntToBoolean, n, delimiter) - val arrayTypeRepr = toArrayTupleType(inferredTypeRepr) - arrayTypeRepr.asType match - case '[arrTup] => constructColumnArrays[hdrs & Tuple, arrTup & Tuple] - end match - - case _ => - report.throwError("Could not infer literal header tuple.") - end match - end if - end match + // Handle dense array modes first. + // + // Wrapped so the *compiler's* handle on the CSV is released however this expansion ends, including the `report.throwError` paths, which unwind through here. Left open it + // leaks one descriptor per expansion for the lifetime of a build daemon - which is measured in days, across every call site in every project that daemon serves. + try + denseColMajorType match + case Some(elemType) => + elemType.asType match + case '[t] => + given Expr[scala.reflect.ClassTag[t]] = Expr.summon[scala.reflect.ClassTag[t]].getOrElse { + report.throwError(s"ClassTag not found for type ${elemType.show}") + } + constructDenseArrayColMajor[t] + case None => + denseRowMajorType match + case Some(elemType) => + elemType.asType match + case '[t] => + given Expr[scala.reflect.ClassTag[t]] = Expr.summon[scala.reflect.ClassTag[t]].getOrElse { + report.throwError(s"ClassTag not found for type ${elemType.show}") + } + constructDenseArrayRowMajor[t] + case None => + // Handle rows or columns mode + if !isColumnMode then + headerTupleExpr match + case '{ $tup: hdrs } => + typeInferrerExpr match + + case '{ TypeInferrer.FromTuple[t]() } => + constructRowIterator[hdrs & Tuple, t & Tuple] + + case '{ TypeInferrer.StringType } => + constructRowIterator[hdrs & Tuple, StringyTuple[hdrs & Tuple] & Tuple] + + case '{ TypeInferrer.FirstRow } => + val inferredTypeRepr = InferrerOps.inferrer(iter, true, delimiter = delimiter) + inferredTypeRepr.asType match + case '[v] => + constructRowIterator[hdrs & Tuple, v & Tuple] + end match + + case '{ TypeInferrer.FromAllRows } => + val inferredTypeRepr = InferrerOps.inferrer(iter, false, Int.MaxValue, delimiter) + inferredTypeRepr.asType match + case '[v] => constructRowIterator[hdrs & Tuple, v & Tuple] + end match + + case '{ TypeInferrer.FirstN(${ Expr(n) }) } => + val inferredTypeRepr = InferrerOps.inferrer(iter, true, n, delimiter) + inferredTypeRepr.asType match + case '[v] => constructRowIterator[hdrs & Tuple, v & Tuple] + end match + + case '{ TypeInferrer.FirstN(${ Expr(n) }, ${ Expr(preferIntToBoolean) }) } => + val inferredTypeRepr = InferrerOps.inferrer(iter, preferIntToBoolean, n, delimiter) + inferredTypeRepr.asType match + case '[v] => constructRowIterator[hdrs & Tuple, v & Tuple] + end match + + case _ => + report.throwError("Could not infer literal header tuple.") + end match + else // isColumnMode + headerTupleExpr match + case '{ $tup: hdrs } => + typeInferrerExpr match + + case '{ TypeInferrer.FromTuple[t]() } => + val arrayTypeRepr = toArrayTupleType(TypeRepr.of[t]) + arrayTypeRepr.asType match + case '[arrTup] => constructColumnArrays[hdrs & Tuple, arrTup & Tuple] + end match + + case '{ TypeInferrer.StringType } => + val stringyType = TypeRepr.of[StringyTuple[hdrs & Tuple]] + val arrayTypeRepr = toArrayTupleType(stringyType) + arrayTypeRepr.asType match + case '[arrTup] => constructColumnArrays[hdrs & Tuple, arrTup & Tuple] + end match + + case '{ TypeInferrer.FirstRow } => + val inferredTypeRepr = InferrerOps.inferrer(iter, true, delimiter = delimiter) + val arrayTypeRepr = toArrayTupleType(inferredTypeRepr) + arrayTypeRepr.asType match + case '[arrTup] => constructColumnArrays[hdrs & Tuple, arrTup & Tuple] + end match + + case '{ TypeInferrer.FromAllRows } => + val inferredTypeRepr = InferrerOps.inferrer(iter, false, Int.MaxValue, delimiter) + val arrayTypeRepr = toArrayTupleType(inferredTypeRepr) + arrayTypeRepr.asType match + case '[arrTup] => constructColumnArrays[hdrs & Tuple, arrTup & Tuple] + end match + + case '{ TypeInferrer.FirstN(${ Expr(n) }) } => + val inferredTypeRepr = InferrerOps.inferrer(iter, true, n, delimiter) + val arrayTypeRepr = toArrayTupleType(inferredTypeRepr) + arrayTypeRepr.asType match + case '[arrTup] => constructColumnArrays[hdrs & Tuple, arrTup & Tuple] + end match + + case '{ TypeInferrer.FirstN(${ Expr(n) }, ${ Expr(preferIntToBoolean) }) } => + val inferredTypeRepr = InferrerOps.inferrer(iter, preferIntToBoolean, n, delimiter) + val arrayTypeRepr = toArrayTupleType(inferredTypeRepr) + arrayTypeRepr.asType match + case '[arrTup] => constructColumnArrays[hdrs & Tuple, arrTup & Tuple] + end match + + case _ => + report.throwError("Could not infer literal header tuple.") + end match + end if + end match + finally source.close() + end try end readHeaderlineAsCsv @@ -990,25 +1003,32 @@ object CSV: */ private inline def fromTyped[K <: Tuple, V <: Tuple](headers: HeaderOptions): PlatformPath => CsvIterator[K, V] = (path: PlatformPath) => - val lines = scala.io.Source.fromFile(path.platformPathString).getLines() - val (hdrs, iterator) = lines.headers(headers) - val expectedHeaders = scala.compiletime.constValueTuple[K].toArray.toSeq.asInstanceOf[Seq[String]] - hdrs.zip(expectedHeaders).zipWithIndex.foreach { case ((a, b), idx) => - if a != b then - throw new IllegalStateException( - s"CSV headers do not match expected headers. Expected: $expectedHeaders, Got: $hdrs. Header mismatch at index $idx: expected '$b', got '$a'" - ) - } - - if hdrs.length != expectedHeaders.length then - throw new IllegalStateException(s"You provided: ${expectedHeaders.size} but ${hdrs.size} headers were found in the file at ${path.platformPathString}.") - end if - - val sizeOfV = scala.compiletime.constValue[Tuple.Size[V]] - if hdrs.length != sizeOfV then - throw new IllegalStateException(s"Number of headers in CSV (${hdrs.length}) does not match number (${sizeOfV}) of types provided for decoding.") - end if + val source = scala.io.Source.fromFile(path.platformPathString) + // Every check below can throw, and each one used to strand the handle it had just opened. + try + val (hdrs, iterator) = source.getLines().headers(headers) + val expectedHeaders = scala.compiletime.constValueTuple[K].toArray.toSeq.asInstanceOf[Seq[String]] + hdrs.zip(expectedHeaders).zipWithIndex.foreach { case ((a, b), idx) => + if a != b then + throw new IllegalStateException( + s"CSV headers do not match expected headers. Expected: $expectedHeaders, Got: $hdrs. Header mismatch at index $idx: expected '$b', got '$a'" + ) + } - new CsvIterator[K, V](iterator, hdrs) + if hdrs.length != expectedHeaders.length then + throw new IllegalStateException(s"You provided: ${expectedHeaders.size} but ${hdrs.size} headers were found in the file at ${path.platformPathString}.") + end if + + val sizeOfV = scala.compiletime.constValue[Tuple.Size[V]] + if hdrs.length != sizeOfV then + throw new IllegalStateException(s"Number of headers in CSV (${hdrs.length}) does not match number (${sizeOfV}) of types provided for decoding.") + end if + + new CsvIterator[K, V](iterator, hdrs, ',', Some(source)) + catch + case e: Throwable => + source.close() + throw e + end try end CSV diff --git a/scautable/src/csvIterator.scala b/scautable/src/csvIterator.scala index 719c6d489..7438cd9b7 100644 --- a/scautable/src/csvIterator.scala +++ b/scautable/src/csvIterator.scala @@ -14,6 +14,9 @@ import scala.compiletime.* * val csvData = csvIterator.toSeq * ``` * + * It owns the file handle it reads from, and releases it when the rows run out - so the shape above needs no cleanup. If you stop reading early, close it yourself, or wrap it in + * `scala.util.Using`; see [[close]]. + * * Note that at this point, you are plugged right into the scala collections API. * * ```scala sc:nocompile @@ -22,15 +25,54 @@ import scala.compiletime.* * etc */ -class CsvIterator[K <: Tuple, V <: Tuple] @publicInBinary private[scautable] (private val rows: Iterator[String], val headers: Seq[String], delimiter: Char = ',')(using +class CsvIterator[K <: Tuple, V <: Tuple] @publicInBinary private[scautable] ( + private val rows: Iterator[String], + val headers: Seq[String], + delimiter: Char = ',', + private val source: Option[AutoCloseable] = None +)(using decoder: RowDecoder[V] -) extends Iterator[NamedTuple[K, V]]: +) extends Iterator[NamedTuple[K, V]] + with AutoCloseable: type COLUMNS = K type Col[N <: Int] = Tuple.Elem[K, N] - override def hasNext: Boolean = rows.hasNext + private var closed = false + + /** Release the file handle behind this iterator. + * + * Idempotent, and safe to call at any point - a closed iterator simply reports no more rows. Draining the iterator calls this for you, so the common shapes (`.toSeq`, + * `.toList`, `foreach`, a fully forced `LazyList`) need nothing. Reach for it explicitly when you stop early, or let `scala.util.Using` do it: + * + * ```scala sc:nocompile + * Using(CSV.absolutePath("big.csv"))(_.take(10).toList) + * ``` + * + * Without either, the handle lives until the JVM notices the iterator is unreachable and reclaims it, which is tied to garbage collection rather than to your reading - so + * enough unclosed reads in a row can still exhaust the process's file descriptors. + */ + override def close(): Unit = + if !closed then + closed = true + source.foreach { s => + try s.close() + catch case _: Exception => () // a handle we cannot release is not worth failing a read over + } + end if + end close + + /** Closes the underlying source as soon as the rows run out, so that the ordinary "read the whole file" shapes do not need a `close` at all. */ + override def hasNext: Boolean = + if closed then false + else + val more = rows.hasNext + if !more then close() + end if + more + end if + end hasNext // inline override def next() = // val str = rows.next() @@ -40,6 +82,9 @@ class CsvIterator[K <: Tuple, V <: Tuple] @publicInBinary private[scautable] (pr // end next override def next(): NamedTuple[K, V] = + // Not just tidiness: past a close the underlying source is gone, so reading on would hand back whatever happened to be buffered, or fail deep inside the reader. + if closed then throw new NoSuchElementException("This CsvIterator has been closed, so there are no more rows to read.") + end if val str = rows.next() val splitted = CSVParser.parseLine(str, delimiter) val tuple = decoder diff --git a/scautable/test/src/IteratorResourceSuite.scala b/scautable/test/src/IteratorResourceSuite.scala new file mode 100644 index 000000000..543497bc0 --- /dev/null +++ b/scautable/test/src/IteratorResourceSuite.scala @@ -0,0 +1,78 @@ +package io.github.quafadas.scautable + +/** A `CsvIterator` owns the handle it reads from. + * + * Before it did, `scala.io.Source` was opened, its line iterator kept, and the `Source` itself dropped on the floor - so nothing ever closed it. The handle came back only when + * the JVM noticed the object was unreachable, which is tied to garbage collection rather than to reading, and 20k reads of a small file could stack up thousands of descriptors + * before any were returned. + * + * These use a counting stand-in for the source, so the assertions are about the iterator's contract rather than about file descriptors, and hold on every platform. + */ +class IteratorResourceSuite extends munit.FunSuite: + + class Spy extends AutoCloseable: + var closes = 0 + override def close(): Unit = closes += 1 + end Spy + + private def iteratorOf(spy: Spy, rows: String*) = + new CsvIterator[("a", "b"), (String, String)](rows.iterator, Seq("a", "b"), ',', Some(spy)) + + test("draining the iterator releases the source") { + val spy = new Spy + val itr = iteratorOf(spy, "1,2", "3,4") + assertEquals(spy.closes, 0, "opening must not close anything") + assertEquals(itr.toList.map(r => (r.a, r.b)), List(("1", "2"), ("3", "4"))) + assertEquals(spy.closes, 1, "running out of rows should close the source") + } + + test("an empty file is closed by the first hasNext") { + val spy = new Spy + val itr = iteratorOf(spy) + assert(!itr.hasNext) + assertEquals(spy.closes, 1) + } + + test("close is idempotent, and closing mid-read ends the iteration") { + val spy = new Spy + val itr = iteratorOf(spy, "1,2", "3,4", "5,6") + assertEquals(itr.next().a, "1") + itr.close() + assertEquals(spy.closes, 1) + assert(!itr.hasNext, "a closed iterator has no more rows") + intercept[NoSuchElementException](itr.next()) + + // exhausting an already closed iterator must not close it twice + itr.close() + assertEquals(spy.closes, 1) + } + + test("closing after a full drain does not close twice") { + val spy = new Spy + val itr = iteratorOf(spy, "1,2") + itr.toList + itr.close() + assertEquals(spy.closes, 1) + } + + test("it is an AutoCloseable, so Using releases it on an early exit") { + val spy = new Spy + val first = scala.util.Using(iteratorOf(spy, "1,2", "3,4", "5,6"))(_.next().a) + assertEquals(first, scala.util.Success("1")) + assertEquals(spy.closes, 1, "Using should close even though only one row was read") + } + + test("Using closes even when the body throws") { + val spy = new Spy + val boom = scala.util.Using(iteratorOf(spy, "1,2"))(_ => throw new RuntimeException("boom")) + assert(boom.isFailure) + assertEquals(spy.closes, 1) + } + + test("an iterator with no source at all still behaves") { + val itr = new CsvIterator[("a", "b"), (String, String)](Iterator("1,2"), Seq("a", "b")) + assertEquals(itr.toList.map(_.a), List("1")) + itr.close() + } + +end IteratorResourceSuite diff --git a/site/docs/csv.md b/site/docs/csv.md index aa23d9bd4..b717b2156 100644 --- a/site/docs/csv.md +++ b/site/docs/csv.md @@ -42,6 +42,30 @@ val csv_root = CSV.projectRoot("data/file.csv", opts) For customisation options look at `CsvOpts`, and supply that as a second argument to any of the above methods. +### Closing the file + +A `CsvIterator` holds the file handle it reads from, and releases it as soon as the rows run +out. The usual shapes therefore need nothing from you: + +```scala sc:nocompile +CSV.resource("simple.csv").toSeq // drained, so closed +CSV.resource("simple.csv").foreach(println) // likewise +``` + +If you stop reading early, the handle stays open until the JVM notices the iterator is +unreachable - which is tied to garbage collection rather than to your reading, so enough +abandoned reads in a row can exhaust the process's file descriptors. Say so explicitly, or +let `Using` do it: + +```scala sc:nocompile +val head = Using(CSV.absolutePath("big.csv"))(_.take(10).toList).get +``` + +Note that `.take(10)` hands back a *new* iterator, which has no way to tell the original it is +finished - that is why the `Using` wraps the `CsvIterator` itself rather than the slice. The +same applies to `JsonTable`; `Excel` shares one cached workbook per file instead, which +`ExcelResourceManager.cleanup` releases. + ## Columnar Reading By default, CSV data is read as an iterator of rows (`CsvIterator`). For analytical workloads, you can read CSV data directly into a columnar format using `ReadAs.Columns`: From 6fcb8d1ec431c6668ed7e8593c859d984ed30704 Mon Sep 17 00:00:00 2001 From: Simon Parten Date: Thu, 1 Oct 2026 10:37:53 +0200 Subject: [PATCH 4/4] Close the compile time CSV handle around header reading too The `finally` added in the previous commit started below the header line, so the two ways reading a header can fail still stranded the compiler's handle: `HeaderOptions.Auto` takes `buffered.head`, which throws on an empty CSV, and `CSVParser.parseLine` can throw on a malformed one. Those are precisely the compiles someone repeats while still getting the file right, so it was the wrong half to leave outside. The `try` now opens immediately after `Source.fromFile`. Measured, by counting handles on the CSV across the compiler's processes while expanding 40 `TypeInferrer.FirstN(5)` call sites: 40 concurrently open before the fix - one per call site, all alive at once - and 0 after. FirstN is the case that needs this most, since inference stops after five lines and leaves the reader parked mid-file with nothing to exhaust it; but FromAllRows depended on it too, because `scala.io.Source` does not close itself when its lines run out. An empty CSV still fails the build, as it should, and now holds no handle while doing so. Co-Authored-By: Claude Opus 5 (1M context) --- scautable/src/csv.scala | 260 ++++++++++++++++++++-------------------- 1 file changed, 133 insertions(+), 127 deletions(-) diff --git a/scautable/src/csv.scala b/scautable/src/csv.scala index fa3a2989c..e621dc6cf 100644 --- a/scautable/src/csv.scala +++ b/scautable/src/csv.scala @@ -423,135 +423,141 @@ object CSV: s"scautable: could not read a CSV at '${pathChain.absolutePath}' while compiling. The file has to exist at compile time so its columns can be typed.${pathChain.anchorHint}" ) ) - val lineIterator: Iterator[String] = source.getLines() - val (headers, iter) = lineIterator.headers(csvHeaders, delimiter) - - if headers.length != headers.distinct.length then report.info("Possible duplicated headers detected.") - end if - - val headerTupleExpr = Expr.ofTupleFromSeq(headers.map(Expr(_))) - - def constructRowIterator[Hdrs <: Tuple: Type, Data <: Tuple: Type]: Expr[CsvIterator[Hdrs, Data]] = - val absolutePathExpr = Expr(pathChain.absolutePath) - val rootRelativeExpr = Expr(pathChain.rootRelativePath) - val resourceNameExpr = Expr(pathChain.resourceName) - val useFallbackExpr = Expr(pathChain.useFallback) - '{ - val source = CSV.openSourceWithFallback($absolutePathExpr, $rootRelativeExpr, $resourceNameExpr, $useFallbackExpr) - val (headers, iterator) = source.getLines().headers($csvHeadersExpr, $delimiterExpr) - // Handed to the iterator rather than dropped, so that draining it - or closing it early - releases the file handle. - new CsvIterator[Hdrs, Data](iterator, headers, $delimiterExpr, Some(source)) - } - end constructRowIterator - - def constructColumnArrays[Hdrs <: Tuple: Type, ArrayData <: Tuple: Type]: Expr[NamedTuple[Hdrs, ArrayData]] = - val absolutePathExpr = Expr(pathChain.absolutePath) - val rootRelativeExpr = Expr(pathChain.rootRelativePath) - val resourceNameExpr = Expr(pathChain.resourceName) - val useFallbackExpr = Expr(pathChain.useFallback) - '{ - val source = CSV.openSourceWithFallback($absolutePathExpr, $rootRelativeExpr, $resourceNameExpr, $useFallbackExpr) - // `finally`, because a malformed line makes `parseLine` throw, and the handle would otherwise be stranded. - val buffers = - try - val (headers, iterator) = source.getLines().headers($csvHeadersExpr, $delimiterExpr) - val numCols = headers.length - val bufs = Array.fill(numCols)(scala.collection.mutable.ArrayBuffer[String]()) - iterator.foreach { line => - val parsed = CSVParser.parseLine(line, $delimiterExpr) - var i = 0 - while i < parsed.length && i < numCols do - bufs(i) += parsed(i) - i += 1 - end while - } - bufs - finally source.close() - end try - end buffers - - val typedColumns = ColumnsDecoder.decodeAllColumns[ArrayData](buffers) - NamedTuple.build[Hdrs & Tuple]()(typedColumns) - } - end constructColumnArrays - - def constructDenseArrayColMajor[T: Type](using - ct: Expr[scala.reflect.ClassTag[T]] - ): Expr[NamedTuple[("data", "rowStride", "colStride", "rows", "cols"), (Array[T], Int, Int, Int, Int)]] = - val absolutePathExpr = Expr(pathChain.absolutePath) - val rootRelativeExpr = Expr(pathChain.rootRelativePath) - val resourceNameExpr = Expr(pathChain.resourceName) - val useFallbackExpr = Expr(pathChain.useFallback) - // Summon the decoder at compile-time - val decoderExpr = Expr.summon[ColumnDecoder[T]].getOrElse { - report.throwError(s"No ColumnDecoder available for type ${Type.show[T]}") - } - val buffersExpr = '{ - val source = CSV.openSourceWithFallback($absolutePathExpr, $rootRelativeExpr, $resourceNameExpr, $useFallbackExpr) - // `finally`, because a malformed line makes `parseLine` throw, and the handle would otherwise be stranded. - val buffers = - try - val (headers, iterator) = source.getLines().headers($csvHeadersExpr, $delimiterExpr) - val numCols = headers.length - val bufs = Array.fill(numCols)(scala.collection.mutable.ArrayBuffer[String]()) - iterator.foreach { line => - val parsed = CSVParser.parseLine(line, $delimiterExpr) - var i = 0 - while i < parsed.length && i < numCols do - bufs(i) += parsed(i) - i += 1 - end while - } - bufs - finally source.close() - end try - end buffers - buffers - } - CSV.buildDenseArrayColMajor[T](buffersExpr, decoderExpr, ct) - end constructDenseArrayColMajor - - def constructDenseArrayRowMajor[T: Type](using - ct: Expr[scala.reflect.ClassTag[T]] - ): Expr[NamedTuple[("data", "rowStride", "colStride", "rows", "cols"), (Array[T], Int, Int, Int, Int)]] = - val absolutePathExpr = Expr(pathChain.absolutePath) - val rootRelativeExpr = Expr(pathChain.rootRelativePath) - val resourceNameExpr = Expr(pathChain.resourceName) - val useFallbackExpr = Expr(pathChain.useFallback) - // Summon the decoder at compile-time - val decoderExpr = Expr.summon[ColumnDecoder[T]].getOrElse { - report.throwError(s"No ColumnDecoder available for type ${Type.show[T]}") - } - val buffersExpr = '{ - val source = CSV.openSourceWithFallback($absolutePathExpr, $rootRelativeExpr, $resourceNameExpr, $useFallbackExpr) - // `finally`, because a malformed line makes `parseLine` throw, and the handle would otherwise be stranded. - val buffers = - try - val (headers, iterator) = source.getLines().headers($csvHeadersExpr, $delimiterExpr) - val numCols = headers.length - val bufs = Array.fill(numCols)(scala.collection.mutable.ArrayBuffer[String]()) - iterator.foreach { line => - val parsed = CSVParser.parseLine(line, $delimiterExpr) - var i = 0 - while i < parsed.length && i < numCols do - bufs(i) += parsed(i) - i += 1 - end while - } - bufs - finally source.close() - end try - end buffers - buffers - } - CSV.buildDenseArrayRowMajor[T](buffersExpr, decoderExpr, ct) - end constructDenseArrayRowMajor - - // Handle dense array modes first. + // Everything that touches the file lives inside this `try`, header line included. // - // Wrapped so the *compiler's* handle on the CSV is released however this expansion ends, including the `report.throwError` paths, which unwind through here. Left open it - // leaks one descriptor per expansion for the lifetime of a build daemon - which is measured in days, across every call site in every project that daemon serves. + // `HeaderOptions.Auto` takes `buffered.head`, which throws on an empty CSV, and `CSVParser.parseLine` can throw on a malformed one - both before any inference starts. The + // handle has to be released on those paths too, and they are exactly the paths a build hits repeatedly while someone is still getting the file right. + // + // `finally` also covers the `report.throwError` exits below: those abort the expansion by throwing, so a close at the end of the happy path alone would be skipped. try + val lineIterator: Iterator[String] = source.getLines() + val (headers, iter) = lineIterator.headers(csvHeaders, delimiter) + + if headers.length != headers.distinct.length then report.info("Possible duplicated headers detected.") + end if + + val headerTupleExpr = Expr.ofTupleFromSeq(headers.map(Expr(_))) + + def constructRowIterator[Hdrs <: Tuple: Type, Data <: Tuple: Type]: Expr[CsvIterator[Hdrs, Data]] = + val absolutePathExpr = Expr(pathChain.absolutePath) + val rootRelativeExpr = Expr(pathChain.rootRelativePath) + val resourceNameExpr = Expr(pathChain.resourceName) + val useFallbackExpr = Expr(pathChain.useFallback) + '{ + val source = CSV.openSourceWithFallback($absolutePathExpr, $rootRelativeExpr, $resourceNameExpr, $useFallbackExpr) + val (headers, iterator) = source.getLines().headers($csvHeadersExpr, $delimiterExpr) + // Handed to the iterator rather than dropped, so that draining it - or closing it early - releases the file handle. + new CsvIterator[Hdrs, Data](iterator, headers, $delimiterExpr, Some(source)) + } + end constructRowIterator + + def constructColumnArrays[Hdrs <: Tuple: Type, ArrayData <: Tuple: Type]: Expr[NamedTuple[Hdrs, ArrayData]] = + val absolutePathExpr = Expr(pathChain.absolutePath) + val rootRelativeExpr = Expr(pathChain.rootRelativePath) + val resourceNameExpr = Expr(pathChain.resourceName) + val useFallbackExpr = Expr(pathChain.useFallback) + '{ + val source = CSV.openSourceWithFallback($absolutePathExpr, $rootRelativeExpr, $resourceNameExpr, $useFallbackExpr) + // `finally`, because a malformed line makes `parseLine` throw, and the handle would otherwise be stranded. + val buffers = + try + val (headers, iterator) = source.getLines().headers($csvHeadersExpr, $delimiterExpr) + val numCols = headers.length + val bufs = Array.fill(numCols)(scala.collection.mutable.ArrayBuffer[String]()) + iterator.foreach { line => + val parsed = CSVParser.parseLine(line, $delimiterExpr) + var i = 0 + while i < parsed.length && i < numCols do + bufs(i) += parsed(i) + i += 1 + end while + } + bufs + finally source.close() + end try + end buffers + + val typedColumns = ColumnsDecoder.decodeAllColumns[ArrayData](buffers) + NamedTuple.build[Hdrs & Tuple]()(typedColumns) + } + end constructColumnArrays + + def constructDenseArrayColMajor[T: Type](using + ct: Expr[scala.reflect.ClassTag[T]] + ): Expr[NamedTuple[("data", "rowStride", "colStride", "rows", "cols"), (Array[T], Int, Int, Int, Int)]] = + val absolutePathExpr = Expr(pathChain.absolutePath) + val rootRelativeExpr = Expr(pathChain.rootRelativePath) + val resourceNameExpr = Expr(pathChain.resourceName) + val useFallbackExpr = Expr(pathChain.useFallback) + // Summon the decoder at compile-time + val decoderExpr = Expr.summon[ColumnDecoder[T]].getOrElse { + report.throwError(s"No ColumnDecoder available for type ${Type.show[T]}") + } + val buffersExpr = '{ + val source = CSV.openSourceWithFallback($absolutePathExpr, $rootRelativeExpr, $resourceNameExpr, $useFallbackExpr) + // `finally`, because a malformed line makes `parseLine` throw, and the handle would otherwise be stranded. + val buffers = + try + val (headers, iterator) = source.getLines().headers($csvHeadersExpr, $delimiterExpr) + val numCols = headers.length + val bufs = Array.fill(numCols)(scala.collection.mutable.ArrayBuffer[String]()) + iterator.foreach { line => + val parsed = CSVParser.parseLine(line, $delimiterExpr) + var i = 0 + while i < parsed.length && i < numCols do + bufs(i) += parsed(i) + i += 1 + end while + } + bufs + finally source.close() + end try + end buffers + buffers + } + CSV.buildDenseArrayColMajor[T](buffersExpr, decoderExpr, ct) + end constructDenseArrayColMajor + + def constructDenseArrayRowMajor[T: Type](using + ct: Expr[scala.reflect.ClassTag[T]] + ): Expr[NamedTuple[("data", "rowStride", "colStride", "rows", "cols"), (Array[T], Int, Int, Int, Int)]] = + val absolutePathExpr = Expr(pathChain.absolutePath) + val rootRelativeExpr = Expr(pathChain.rootRelativePath) + val resourceNameExpr = Expr(pathChain.resourceName) + val useFallbackExpr = Expr(pathChain.useFallback) + // Summon the decoder at compile-time + val decoderExpr = Expr.summon[ColumnDecoder[T]].getOrElse { + report.throwError(s"No ColumnDecoder available for type ${Type.show[T]}") + } + val buffersExpr = '{ + val source = CSV.openSourceWithFallback($absolutePathExpr, $rootRelativeExpr, $resourceNameExpr, $useFallbackExpr) + // `finally`, because a malformed line makes `parseLine` throw, and the handle would otherwise be stranded. + val buffers = + try + val (headers, iterator) = source.getLines().headers($csvHeadersExpr, $delimiterExpr) + val numCols = headers.length + val bufs = Array.fill(numCols)(scala.collection.mutable.ArrayBuffer[String]()) + iterator.foreach { line => + val parsed = CSVParser.parseLine(line, $delimiterExpr) + var i = 0 + while i < parsed.length && i < numCols do + bufs(i) += parsed(i) + i += 1 + end while + } + bufs + finally source.close() + end try + end buffers + buffers + } + CSV.buildDenseArrayRowMajor[T](buffersExpr, decoderExpr, ct) + end constructDenseArrayRowMajor + + // Handle dense array modes first. + // + // Wrapped so the *compiler's* handle on the CSV is released however this expansion ends, including the `report.throwError` paths, which unwind through here. Left open it + // leaks one descriptor per expansion for the lifetime of a build daemon - which is measured in days, across every call site in every project that daemon serves. denseColMajorType match case Some(elemType) => elemType.asType match