Skip to content

API: Fix misleading error message in predicate()#16872

Open
thswlsqls wants to merge 2 commits into
apache:mainfrom
thswlsqls:fix/predicate-error-message
Open

API: Fix misleading error message in predicate()#16872
thswlsqls wants to merge 2 commits into
apache:mainfrom
thswlsqls:fix/predicate-error-message

Conversation

@thswlsqls

@thswlsqls thswlsqls commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Expressions.predicate(Operation, String, Literal) rejects the unary operations IS_NULL, NOT_NULL, IS_NAN, NOT_NAN (which take no value) with the message Cannot create %s predicate inclusive a value.
  • inclusive a value is not grammatical and does not describe the failure.
  • The sibling method predicate(Operation, String) rejects value-taking operations with Cannot create %s predicate without a value. This is the mirror case — a value passed to a unary op — so the message should read with a value.
  • Fix: change the literal to Cannot create %s predicate with a value. No signature, branch, or ABI change.

Testing done

  • Added TestExpressionHelpers#testPredicateWithValueForUnaryOperation asserting predicate(IS_NULL, "x", 5) throws IllegalArgumentException with message Cannot create IS_NULL predicate with a value.
  • ./gradlew :iceberg-api:check — passed (TestExpressionHelpers: 14 tests, 0 failures; includes spotlessCheck, checkstyle, errorProne).
  • ./gradlew :iceberg-api:revapi — passed (backward compatible; only an error-message string changed).

AI Disclosure

  • Model: Claude Opus 4.8
  • Platform/Tool: Claude Code

The Preconditions check in predicate(Operation, String, Literal) rejected
unary operations (IS_NULL, NOT_NULL, IS_NAN, NOT_NAN) that take no value
with the message "Cannot create %s predicate inclusive a value", which is
not grammatical. Change it to "with a value" so it mirrors the sibling
method predicate(Operation, String), which rejects value-taking operations
with "without a value".

Generated-by: Claude Code
@github-actions github-actions Bot added the API label Jun 19, 2026
Comment thread api/src/test/java/org/apache/iceberg/expressions/TestExpressionHelpers.java Outdated
Per review feedback, move testPredicateWithValueForUnaryOperation from
TestExpressionHelpers to TestExpressionUtil, where the predicate() factory
is exercised. The assertion is unchanged.

Generated-by: Claude Code
@thswlsqls thswlsqls requested a review from ebyhr June 21, 2026 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants