[
https://issues.apache.org/jira/browse/CALCITE-4085?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144724#comment-17144724
]
Dawid Wysakowicz commented on CALCITE-4085:
-------------------------------------------
I can't see how the [1] can be of any help in my use case. It's just a helper
method around {{RelDataTypeFactory#createTypeWithNullability}}. I don't have
any problems with making a record type nullable. The problem is with preserving
the nullability of nested fields. Because
{{RelDataTypeFactory#createTypeWithNullability}} changes the nested types.
Calling [1] on a type {{USER(id NOT NULL) NOT NULL}} will change it to
{{USER(id <NULLABLE>) <NULLABLE>}}. As I pointed out in my email I have a way
to workaround that in the Flink code. What I can't do in Flink in a reasonable
way is to keep the _"[A].[B]", if [A] is nullable then [A].[B] should also be
nullable_ logic.
??I still think the nullability propagation in type factory is correct. Based
on the SQL standard and the fact that:
for expression "[A].[B]", if [A] is nullable then [A].[B] should also be
nullable.??
On this topic I agree we disagree ;) Personally I think this is the
characteristic of the operator (SqlDotOperator, SqlItemOperator,
RexFieldAccess) not the type itself. The same way as it is property of the
{{AND}} operator that it returns nullable type if any of the operands is
nullable. You do not fix the {{BOOLEAN}} type in typeFactory to be always
nullable, because of the {{AND}} operator. Nevertheless because as I said there
is a way to override that behaviour in our type factory in a reasonable way, I
don't want to insist on changing that behaviour in Calcite. What I'd really
appreciate is to have the logic in the operators as well, as they can not be
overriden in a good way (without copying over the classes).
Yes, I agree with your analysis of the {{TableFunctionTest#testTableFunction}}
but for me it shows that either:
* the mapping from POJO to JavaType is wrong
* the logic in [2] is wrong
If you think adding an exclusion for the {{JavaType}} is a right solution
there, I can do that. I will update the PR shortly. Personally though I don't
like it as it looks like a hack for me. I don't necessarily understand how are
the {{JavaTypes}} different in this case than regular sql types.
Lastly, I'd like to ask again about the changes in
{{RexBuilder#makeFieldAccessInternal}} and
{{SqlValidatorImpl.DeriveTypeVisitor#visit(SqlIdentifier)}}. Those are
equivalents of {{SqlDotOperator}} when we are resolving a field access like:
{code}
CREATE TYPE User (
id INT NOT NULL
);
CREATE TABLE Accounts (
user USER -- nullable
)
SELECT t.user.id FROM Accounts t;
{code}
What would be your suggestion how can I integrate the same fix as for
{{SqlDotOperator}} and {{SqlItemOperator}} in this case? As mentioned before in
this ticket and also in my email I can override that in an acceptable way in
our codebase, but would also appreciate a built-in support for it. Any
suggestions on that?
[1]
https://github.com/apache/calcite/blob/0769a8b31cbbeb5bca66ade30cf3710523da4aaa/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactory.java#L518
[2]
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)
