Skip to content

🐛 fix: surface UnexpectedNullableFound instead of remapping to ColumnNotFound (closes #560) #712

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MavenRain wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🐛 fix: surface UnexpectedNullableFound instead of remapping to ColumnNotFound (closes #560) #712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions core/src/main/scala/anorm/RowParser.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Copy Markdown
Member

@gaeljw gaeljw May 19, 2026

Choose a reason for hiding this comment

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

I don't think we need this entire comment. Sounds like classic AI over commenting ;)

// 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)
Copy link
Copy Markdown
Member

@cchantep cchantep May 19, 2026

Choose a reason for hiding this comment

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

If that's the desired behaviour, the pattern could be merge with the previous case


case e @ Error(_) => e
}
Expand Down
23 changes: 8 additions & 15 deletions core/src/main/scala/anorm/SqlParser.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Copy Markdown
Member

@gaeljw gaeljw May 19, 2026

Choose a reason for hiding this comment

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

Same comment, I don't think the comment is useful.

// `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(_))
Expand Down Expand Up @@ -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(_))
}
Expand All @@ -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(_))
}
Expand All @@ -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")
Expand Down
15 changes: 15 additions & 0 deletions core/src/test/scala/AnormSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading