Fix ColumnSignature error message and jdk17 test issue.#14538
Fix ColumnSignature error message and jdk17 test issue.#14538clintropolis merged 2 commits intoapache:masterfrom
Conversation
On jdk17, the "problem" part of the error message could change from NullPointerException to: Cannot invoke "String.length()" because "s" is null Due to the new more-helpful NPEs in Java 17. This broke the expectation and led to test failures on this case. This patch fixes the problem by improving the error message so it isn't a generic NullPointerException.
|
The test was added in #14531. I'm not sure how jdk17 CI passed for that version. It certainly failed on another PR here: https://github.com/apache/druid/actions/runs/5471321754/jobs/9978180802?pr=14532. Maybe it's flaky; perhaps the helpful NPE message isn't always generated. |
| // Name must be nonnull, but type can be null (if the type is unknown) | ||
| if (name == null || name.isEmpty()) { | ||
| throw new IAE(name, "Column name must be non-empty"); | ||
| throw new IAE("Column name must be provided and non-empty"); |
There was a problem hiding this comment.
Drive by comment, we could've interpolated the name value here:
"Column name [%s] must be provided and non-empty", name
Would help differentiate between empty and null. That said, I wonder if this should be throwing an InvalidInput instead of IAE now.
There was a problem hiding this comment.
It should probably have been doing InvalidInput. I forgot about those! Somehow!
|
I pulled master and ran the test on jdk17 in sql compatible mode, it is failing the test. |
* Fix ColumnSignature error message and jdk17 test issue. On jdk17, the "problem" part of the error message could change from NullPointerException to: Cannot invoke "String.length()" because "s" is null Due to the new more-helpful NPEs in Java 17. This broke the expectation and led to test failures on this case. This patch fixes the problem by improving the error message so it isn't a generic NullPointerException. * Fix format.
6 participants
On jdk17, the "problem" part of the error message could change from NullPointerException to:
Due to the new more-helpful NPEs in Java 17. This broke the expectation and led to test failures on this case.
This patch fixes the problem by improving the error message so it isn't a generic NullPointerException.