Hi,

I suggest we add nullness verification to the main (non-test) code.
That would make the code easier to follow, and it would avoid NPEs.
checkerframework is an established verification framework for Java, and
they do ~monthly releases ( https://checkerframework.org/changelog.txt )

The verification for linq4j and core takes ~10min, so I guess we could run
it for each PR.

Frankly speaking, I'm quite happy with the results.
The overall idea is "assume all references (except local vars) are
non-nullable, then @Nullable can be added when needed".

The more I work with it the more I like it. @Nullable annotations make it
immediately visible if the field/parameter/result value can be null or not.

Recently Kenneth shared their strongly-positive experience when adding
checkerframework to Beam
https://lists.apache.org/thread.html/rb4991ed24668b0523fe9ac0ef185b471f5a8171655550138a674c407%40%3Cdev.calcite.apache.org%3E

I wonder what do you think?

Even though the patch looks big, I rebase it from time to time, and the
rare conflicts are easy to resolve.

---

The corresponding issue is
https://issues.apache.org/jira/browse/CALCITE-4199 , and it has been open
for a while, however, now I guess
the patch is more-or-less ready for the review.
Note: there are still nullability violations left, however, I think the
most part of the significant changes is already implemented.

Note: there are lots of sub-tasks under 4199, and many of them are not that
trivial.
For instance, RelNode#estimateRowCount must return non-null int, and
metadata returns null for "unknown rowcount" case. Apparently, that might
fail with NPE,
and it is not clear how to approach that.

The PR is https://github.com/apache/calcite/pull/1929
Note: I moved "add @Nullable annotations"  to the separate commits (the
commit messages end with "@nullable annotations only").

For instance,
linq4j refactor:
https://github.com/apache/calcite/pull/1929/commits/44628938ef723547dbb1a0aaa4c3233b724bba1d
core refactor:
https://github.com/apache/calcite/pull/1929/commits/31e17a09c45c555ac07dc3f82ee06ce8b1108ef8

Vladimir

Reply via email to