Nikolay>Do we have some code style convention that requires it?
Even existing code convention is to explain if parameter can be nullable or
not.
Often it is explained in javadoc or requireNotNull(...) call.
The PR enforces that convention. For instance, it forbids (build would
fail) to return nulls if the return type is non-nullable.
Nikolay>Every java reference variable can be null.
I'm afraid you are not quite right.
If every reference is nullable, then the code must be "if (null)" all over
the place.
Calcite uses Guava's Immutable collections heavily, and the collections
never permit nulls.
For instance, JoinRel has 5 fields non-nullable, so no annotations needed.
public abstract class Join extends BiRel implements Hintable {
protected final RexNode condition;
protected final ImmutableSet<CorrelationId> variablesSet;
protected final ImmutableList<RelHint> hints;
protected final JoinRelType joinType;
protected final JoinInfo joinInfo;
Here are the most @nullable-heavy files.
"number of lines with @Nullable" "total lines" "file"
95 6958 src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
70 6248 src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
60 3675 src/main/java/org/apache/calcite/tools/RelBuilder.java
59 4573 src/main/java/org/apache/calcite/plan/RelOptUtil.java
43 1144 src/main/java/org/apache/calcite/adapter/enumerable/EnumUtils.java
42 913 src/main/java/org/apache/calcite/adapter/clone/ArrayTable.java
40 276 src/main/java/org/apache/calcite/sql/SqlSelect.java
38 419 src/main/java/org/apache/calcite/rel/metadata/RelMdSize.java
37 821 src/main/java/org/apache/calcite/runtime/JsonFunctions.java
36 559 src/main/java/org/apache/calcite/rel/metadata/RelMdCollation.java
35 696 src/main/java/org/apache/calcite/rel/metadata/BuiltInMetadata.java
33 3013 src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
33 1602 src/main/java/org/apache/calcite/sql/SqlDialect.java
33 1419
src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewRule.java
32 881 src/main/java/org/apache/calcite/rel/metadata/RelMetadataQuery.java
31 547 src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java
30 1357 src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java
I would say the null density is very far from your "every java reference".
So the convention is to assume nulls are not permitted, and mark nullable
types when nulls are needed.
Nikolay>Can you, please, show the list of the bugs founded
Please find the samples I logged as sub-tickets:
https://issues.apache.org/jira/browse/CALCITE-4199
Note: that is not the full set of issues. My initial plan was to log every
issue, discuss it and resolve it.
However, later I realized the tickets don't get enough attention, so the
plan to log them all and fix them is not really viable.
That is why I kept NPE in many places which otherwise would discussions to
resolve.
Vladimir
