From a17fe1396f4ecca70ff90956080c617f6cc26f2d Mon Sep 17 00:00:00 2001 From: Onyeka Obi Date: Tue, 19 May 2026 09:01:06 -0700 Subject: [PATCH] =?UTF-8?q?=20=20=F0=9F=90=9B=20fix:=20surface=20Unexpecte?= =?UTF-8?q?dNullableFound=20instead=20of=20remapping=20to=20ColumnNotFound?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, NULL columns triggered an `UnexpectedNullableFound` error from `Column[T]`, but `SqlParser`'s internal `parseColumn` helper remapped it to `ColumnNotFound`, producing misleading "column not found" errors like "'X' not found, available columns: X, X" where the column was actually present and merely contained NULL. The fix drops the remap, and the now-trivial `parseColumn` helper is inlined at its three call sites. The `.?` combinator on `RowParser` previously depended on the remap to convert NULL to `None`; it now recovers from `UnexpectedNullableFound` directly, honouring its documented "missing or null column as None" contract. Tests: - AnormSpec: scalar[String].single on NULL must surface UnexpectedNullableFound, not "not found, available columns". - SqlResultSpec: int("n").? on NULL returns None (parity with the existing str("foo").? coverage, on the Column[Int] codepath; mirrors the coverage added in #665). Closes #560. Signed-off-by: Onyeka Obi --- core/src/main/scala/anorm/RowParser.scala | 12 +++++++--- core/src/main/scala/anorm/SqlParser.scala | 23 +++++++------------ core/src/test/scala/AnormSpec.scala | 15 ++++++++++++ core/src/test/scala/anorm/SqlResultSpec.scala | 9 ++++++++ 4 files changed, 41 insertions(+), 18 deletions(-) diff --git a/core/src/main/scala/anorm/RowParser.scala b/core/src/main/scala/anorm/RowParser.scala index 8c6c0a50..d74543b6 100644 --- a/core/src/main/scala/anorm/RowParser.scala +++ b/core/src/main/scala/anorm/RowParser.scala @@ -97,10 +97,16 @@ trait RowParser[+A] extends (Row => SqlResult[A]) { parent => * that will turn missing or null column as None. */ def ? : RowParser[Option[A]] = RowParser { + // Note: `UnexpectedNullableFound` is the error raised by `Column` when a + // non-nullable column is read but contains NULL. Previously the SqlParser + // remapped this to `ColumnNotFound` so that this combinator surfaced NULL + // as `None`; that remap also caused #560 (misleading "column not found" + // errors elsewhere). Now we recover from both errors here directly, which + // is what the scaladoc above already promised. parent(_) match { - case Success(a) => Success(Some(a)) - case Error(ColumnNotFound(_, _)) => - Success(None) + case Success(a) => Success(Some(a)) + case Error(ColumnNotFound(_, _)) => Success(None) + case Error(UnexpectedNullableFound(_)) => Success(None) case e @ Error(_) => e } diff --git a/core/src/main/scala/anorm/SqlParser.scala b/core/src/main/scala/anorm/SqlParser.scala index 286f5390..28ac7096 100644 --- a/core/src/main/scala/anorm/SqlParser.scala +++ b/core/src/main/scala/anorm/SqlParser.scala @@ -29,8 +29,12 @@ object SqlParser extends FunctionAdapter with DeprecatedSqlParser { } yield v -> m).toRight(NoColumnsInReturnedResult) val parsed = Compat.rightFlatMap[SqlMappingError, SqlRequestError, (Any, MetaDataItem), T](input) { - case in @ (_, m) => - parseColumn(row, m.column.qualified, c, in) + case in @ (_, _) => + // Previously a `parseColumn` helper mapped `UnexpectedNullableFound` to + // `ColumnNotFound` here, producing misleading "column not found" errors + // when the column existed but contained NULL (#560). Surface the + // original `Column` error instead. + c.tupled(in) } parsed.fold(Error(_), Success(_)) @@ -491,7 +495,7 @@ object SqlParser extends FunctionAdapter with DeprecatedSqlParser { row => Compat .rightFlatMap(row.get(name)) { in => - parseColumn(row, name, c, in) + c.tupled(in) } .fold(Error(_), Success(_)) } @@ -514,7 +518,7 @@ object SqlParser extends FunctionAdapter with DeprecatedSqlParser { RowParser { row => Compat .rightFlatMap(row.getIndexed(position - 1)) { in => - parseColumn(row, in._2.column.qualified, c, in) + c.tupled(in) } .fold(Error(_), Success(_)) } @@ -536,17 +540,6 @@ object SqlParser extends FunctionAdapter with DeprecatedSqlParser { def matches[T: Column](column: String, value: T): RowParser[Boolean] = get[T](column).?.map(_.fold(false) { _ == value }) - @inline private def parseColumn[T]( - row: Row, - name: String, - c: Column[T], - input: (Any, MetaDataItem) - ): Either[SqlRequestError, T] = c.tupled(input).left.map { - case UnexpectedNullableFound(_) => - ColumnNotFound(name, row) - - case cause => cause - } } @deprecated("Do not use these combinators", "2.5.4") diff --git a/core/src/test/scala/AnormSpec.scala b/core/src/test/scala/AnormSpec.scala index 5d22ec0b..2b7c8d55 100644 --- a/core/src/test/scala/AnormSpec.scala +++ b/core/src/test/scala/AnormSpec.scala @@ -66,6 +66,21 @@ final class AnormSpec } + // Regression test for #560: previously this surfaced as + // "'X' not found, available columns: X, X" because parseColumn mapped + // UnexpectedNullableFound to ColumnNotFound. After the fix it must + // mention the column is NULL, not missing. + "surface NULL-aware error when scalar[T].single hits NULL" in withQueryResult(null.asInstanceOf[String]) { + implicit c: Connection => + + SQL("SELECT * FROM test").as(scalar[String].single).aka("scalar single on NULL") must throwA[Exception] + .like { + case e: Exception => + (e.getMessage.aka("error") must contain("UnexpectedNullableFound")) + .and(e.getMessage.aka("error") must not(contain("not found, available columns"))) + } + } + "throw exception when single result is missing" in withQueryResult(fooBarTable) { implicit c: Connection => SQL("SELECT * FROM test").as(fooBarParser1.single).aka("mapping") must throwA[Exception].like { diff --git a/core/src/test/scala/anorm/SqlResultSpec.scala b/core/src/test/scala/anorm/SqlResultSpec.scala index a349ddd6..2cc9f7d5 100644 --- a/core/src/test/scala/anorm/SqlResultSpec.scala +++ b/core/src/test/scala/anorm/SqlResultSpec.scala @@ -161,6 +161,15 @@ final class SqlResultSpec extends org.specs2.mutable.Specification with H2Databa SQL"SELECT *".as(SqlParser.str("foo").?.single) must beNone } + + // Parallel coverage on the Column[Int] codepath: with the parseColumn + // remap removed, `.?` must still surface a NULL integer as None via the + // new UnexpectedNullableFound branch in RowParser.?. + "be None when column is NULL for non-nullable Int" in withQueryResult( + rowList1(classOf[Integer] -> "n") :+ null.asInstanceOf[Integer] + ) { implicit c: Connection => + SQL"SELECT *".as(SqlParser.int("n").?.single) must beNone + } } "Collecting" should {