[
https://issues.apache.org/jira/browse/CALCITE-4085?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17143625#comment-17143625
]
Dawid Wysakowicz commented on CALCITE-4085:
-------------------------------------------
My end goal is to be able to have a nullable record with not null fields. E.g.
{{USER(id NOT NULL) <NULLABLE>}}. That is because we might have a UDF that
expects {{USER(id NOT NULL) <NULLABLE>}}. From that point of view the logic in
RelDataTypeFactoryImpl[1] is incorrect as it changes the nullability of nested
fields.
I fully agree with "For expression "[A].[B]", if [A] is nullable then [A].[B]
should also be nullable". I don't think though this should be fixed at the time
when the RecordType is created, as it makes, in my opinion, a valid use case
described in the first paragraph impossible. This logic in my
understanding/opinion is very similar if not exactly the same as
{{SqlTypeTransforms#TO_NULLABLE}}, and it does belong to every operator that
accesses nested fields. Of course you cannot prevent people to add new
operators, but again in my opinion this belongs to the operator logic the same
as the mentioned {{SqlTypeTransforms#TO_NULLABLE}}.
This is incorrect:
> You want the int type to be nullable. But in the POJO code, you want to
> translate the id to int instead of integer. In my point, the type inference
> of current Calcite is very close to correct. What we should do is the QE
> (when we translate the POJO, to do a promotion in the code instead) which
> seems more feasible.
I want the int to be {{NOT NULL}} and be translated to {{int}}. Calcite will
convert it to {{NULLABLE}} because of the logic in [1].
>From my perspective changes in RexBuilder#makeFieldAccessInternal,
>SqlValidatorImpl.DeriveTypeVisitor#visit(SqlIdentifier) are necessary because
>I don't want the logic in the type factory as it breaks my use case from the
>first paragraph.
The changes in RexBuilder and SqlValidatorImpl are required for a query like
{{SELECT u.nullable_row.not_null_nested_field FROM
tableWithNullableRowColumn}}, because those do not use SqlDotOperator, but
create a {{RexFieldAccess}}. I split the PR into three commits because I do
understand the changes in RexBuilder#makeFieldAccessInternal,
SqlValidatorImpl.DeriveTypeVisitor#visit(SqlIdentifier) are quite invasive and
you might not agree to incorporate them. So if you are strongly against those I
will just drop those from the PR.
I'd really like to see the other two commits merged. Still the changes in
{{AliasNamespace}} make tests in {{TableFunctionTest}} fail imo because of the
logic in [1]. The tests there join with lateral table generated from a UDF
which return type is {{IntString(n INT NOT NULL, s String) <NULLABLE>}}. If I
change the {{AliasNamespace}} to keep the original nullability(which imo is
correct, I don't see a reason why aliasing should change nullability), then the
logic in [1] changes the nullability of n which in turn causes mismatch.
[1]
https://github.com/apache/calcite/blob/0769a8b31cbbeb5bca66ade30cf3710523da4aaa/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java#L338
> Improve nullability support for fields of structured type
> ---------------------------------------------------------
>
> Key: CALCITE-4085
> URL: https://issues.apache.org/jira/browse/CALCITE-4085
> Project: Calcite
> Issue Type: Improvement
> Components: core
> Reporter: Dawid Wysakowicz
> Assignee: Dawid Wysakowicz
> Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> As discussed in
> https://lists.apache.org/thread.html/r602ac95fff23dd1ef974fb396df7be061ab861384ec42f5c57ce0bc2%40%3Cdev.calcite.apache.org%3E
> I would like to change the way a type of a field of a record is derived at
> couple of locations. This helps frameworks such as Apache Flink to build
> support for nullable records with not null fields.
> I suggest to change:
> * SqlDotOperator#deriveType
> * SqlItemOperator#inferReturnType
> * AliasNamespace#validateImpl
> * RexBuilder#makeFieldAccessInternal
> * SqlValidatorImpl.DeriveTypeVisitor#visit(SqlIdentifier)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
