[jira] [Commented] (CALCITE-3029) Java-oriented field type is wrongly forced to be NOT NULL after being converted to SQL-oriented
[
https://issues.apache.org/jira/browse/CALCITE-3029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16829913#comment-16829913
]
Hongze Zhang commented on CALCITE-3029:
---
I've updated the PR accordingly.
> Java-oriented field type is wrongly forced to be NOT NULL after being
> converted to SQL-oriented
> ---
>
> Key: CALCITE-3029
> URL: https://issues.apache.org/jira/browse/CALCITE-3029
> Project: Calcite
> Issue Type: Bug
>Affects Versions: 1.19.0
>Reporter: Hongze Zhang
>Assignee: Hongze Zhang
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.20.0
>
> Time Spent: 1h 50m
> Remaining Estimate: 0h
>
> A Java-oriented field type loses its nullable constraint after calling method
> {{org.apache.calcite.jdbc.JavaTypeFactoryImpl#toSql(org.apache.calcite.rel.type.RelDataType)}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3029) Java-oriented field type is wrongly forced to be NOT NULL after being converted to SQL-oriented
[
https://issues.apache.org/jira/browse/CALCITE-3029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16829779#comment-16829779
]
Stamatis Zampetakis commented on CALCITE-3029:
--
I agree with all of you; let's remove the flag. I don't think this optim is
worth spending more time on it. Thanks for catching this [~zhztheplayer] and
[~rubenql], [~danny0405] for reviewing ;)
> Java-oriented field type is wrongly forced to be NOT NULL after being
> converted to SQL-oriented
> ---
>
> Key: CALCITE-3029
> URL: https://issues.apache.org/jira/browse/CALCITE-3029
> Project: Calcite
> Issue Type: Bug
>Affects Versions: 1.19.0
>Reporter: Hongze Zhang
>Assignee: Hongze Zhang
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.20.0
>
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> A Java-oriented field type loses its nullable constraint after calling method
> {{org.apache.calcite.jdbc.JavaTypeFactoryImpl#toSql(org.apache.calcite.rel.type.RelDataType)}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3029) Java-oriented field type is wrongly forced to be NOT NULL after being converted to SQL-oriented
[
https://issues.apache.org/jira/browse/CALCITE-3029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16829044#comment-16829044
]
Danny Chan commented on CALCITE-3029:
-
+1 to remove this flag, we should sync the nullability even though it is a
RelRecordType, after all, correctness is the first thing we should take.
> Java-oriented field type is wrongly forced to be NOT NULL after being
> converted to SQL-oriented
> ---
>
> Key: CALCITE-3029
> URL: https://issues.apache.org/jira/browse/CALCITE-3029
> Project: Calcite
> Issue Type: Bug
>Affects Versions: 1.19.0
>Reporter: Hongze Zhang
>Assignee: Hongze Zhang
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.20.0
>
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> A Java-oriented field type loses its nullable constraint after calling method
> {{org.apache.calcite.jdbc.JavaTypeFactoryImpl#toSql(org.apache.calcite.rel.type.RelDataType)}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3029) Java-oriented field type is wrongly forced to be NOT NULL after being converted to SQL-oriented
[
https://issues.apache.org/jira/browse/CALCITE-3029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16829033#comment-16829033
]
Ruben Quesada Lopez commented on CALCITE-3029:
--
{{mustSetNullability }} flag was introduced in CALCITE-2464 with the intention
of optimizing recursive toSql calls when dealing with nested RelRecordTypes,
but as pointed out by [~zhztheplayer], it leads to issues. I think we should
probably remove it at all:
{code:java}
/** Converts a type in Java format to a SQL-oriented type. */
public static RelDataType toSql(final RelDataTypeFactory typeFactory,
RelDataType type) {
RelDataType sqlType = type;
if (type instanceof RelRecordType) {
sqlType = typeFactory.createStructType(
Lists.transform(type.getFieldList(),
field -> toSql(typeFactory, field.getType())),
type.getFieldNames());
} else if (type instanceof JavaType) {
sqlType = typeFactory.createSqlType(type.getSqlTypeName());
}
return typeFactory.createTypeWithNullability(sqlType, type.isNullable());
}
{code}
What do you think, [~zabetak]?
> Java-oriented field type is wrongly forced to be NOT NULL after being
> converted to SQL-oriented
> ---
>
> Key: CALCITE-3029
> URL: https://issues.apache.org/jira/browse/CALCITE-3029
> Project: Calcite
> Issue Type: Bug
>Affects Versions: 1.19.0
>Reporter: Hongze Zhang
>Assignee: Hongze Zhang
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.20.0
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> A Java-oriented field type loses its nullable constraint after calling method
> {{org.apache.calcite.jdbc.JavaTypeFactoryImpl#toSql(org.apache.calcite.rel.type.RelDataType)}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3029) Java-oriented field type is wrongly forced to be NOT NULL after being converted to SQL-oriented
[
https://issues.apache.org/jira/browse/CALCITE-3029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16829015#comment-16829015
]
Hongze Zhang commented on CALCITE-3029:
---
Thanks [~rubenql] and [~zabetak]!
And the PR is currently just a patch for my personal use. Since I didn't dive
into the details of 2464, if there is any better solution you prefer to do or
be confident with, feel free to take the test cases and make your own changes.
:)
> Java-oriented field type is wrongly forced to be NOT NULL after being
> converted to SQL-oriented
> ---
>
> Key: CALCITE-3029
> URL: https://issues.apache.org/jira/browse/CALCITE-3029
> Project: Calcite
> Issue Type: Bug
>Affects Versions: 1.19.0
>Reporter: Hongze Zhang
>Assignee: Hongze Zhang
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.20.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> A Java-oriented field type loses its nullable constraint after calling method
> {{org.apache.calcite.jdbc.JavaTypeFactoryImpl#toSql(org.apache.calcite.rel.type.RelDataType)}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3029) Java-oriented field type is wrongly forced to be NOT NULL after being converted to SQL-oriented
[
https://issues.apache.org/jira/browse/CALCITE-3029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16828991#comment-16828991
]
Ruben Quesada Lopez commented on CALCITE-3029:
--
[~zhztheplayer], I'll take a look.
> Java-oriented field type is wrongly forced to be NOT NULL after being
> converted to SQL-oriented
> ---
>
> Key: CALCITE-3029
> URL: https://issues.apache.org/jira/browse/CALCITE-3029
> Project: Calcite
> Issue Type: Bug
>Affects Versions: 1.19.0
>Reporter: Hongze Zhang
>Assignee: Hongze Zhang
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.20.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> A Java-oriented field type loses its nullable constraint after calling method
> {{org.apache.calcite.jdbc.JavaTypeFactoryImpl#toSql(org.apache.calcite.rel.type.RelDataType)}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3029) Java-oriented field type is wrongly forced to be NOT NULL after being converted to SQL-oriented
[
https://issues.apache.org/jira/browse/CALCITE-3029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16828034#comment-16828034
]
Stamatis Zampetakis commented on CALCITE-3029:
--
OK sorry about that! Either me or [~rubenql] will look into this.
> Java-oriented field type is wrongly forced to be NOT NULL after being
> converted to SQL-oriented
> ---
>
> Key: CALCITE-3029
> URL: https://issues.apache.org/jira/browse/CALCITE-3029
> Project: Calcite
> Issue Type: Bug
>Affects Versions: 1.19.0
>Reporter: Hongze Zhang
>Assignee: Hongze Zhang
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.20.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> A Java-oriented field type loses its nullable constraint after calling method
> {{org.apache.calcite.jdbc.JavaTypeFactoryImpl#toSql(org.apache.calcite.rel.type.RelDataType)}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3029) Java-oriented field type is wrongly forced to be NOT NULL after being converted to SQL-oriented
[
https://issues.apache.org/jira/browse/CALCITE-3029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16828032#comment-16828032
]
Hongze Zhang commented on CALCITE-3029:
---
Thanks [~zabetak].
Regarding the issue, I am more or less sure that the problem didn't exist
before CALCITE-2464. I run the same test case[1] of inserting nullable values
under commit
[8eb852|https://github.com/apache/calcite/commit/8eb852039db04c132ae7a99943495f87cf39dfd2]
(the commit right behind 2464 fix) and it passed.
[1]
[https://github.com/apache/calcite/pull/1187/files#diff-fcafae074e093e34d065f7f5a49f5124R712]
> Java-oriented field type is wrongly forced to be NOT NULL after being
> converted to SQL-oriented
> ---
>
> Key: CALCITE-3029
> URL: https://issues.apache.org/jira/browse/CALCITE-3029
> Project: Calcite
> Issue Type: Bug
>Affects Versions: 1.19.0
>Reporter: Hongze Zhang
>Assignee: Hongze Zhang
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.20.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> A Java-oriented field type loses its nullable constraint after calling method
> {{org.apache.calcite.jdbc.JavaTypeFactoryImpl#toSql(org.apache.calcite.rel.type.RelDataType)}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3029) Java-oriented field type is wrongly forced to be NOT NULL after being converted to SQL-oriented
[
https://issues.apache.org/jira/browse/CALCITE-3029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16828019#comment-16828019
]
Stamatis Zampetakis commented on CALCITE-3029:
--
Thanks for bringing this up [~zhztheplayer]! I think I understand why it
happens, I will try to have a look in the following days. Are you sure that
this is a regression? I was thinking that even before CALCITE-2464 the toSql
method was doing the same thing.
> Java-oriented field type is wrongly forced to be NOT NULL after being
> converted to SQL-oriented
> ---
>
> Key: CALCITE-3029
> URL: https://issues.apache.org/jira/browse/CALCITE-3029
> Project: Calcite
> Issue Type: Bug
>Affects Versions: 1.19.0
>Reporter: Hongze Zhang
>Assignee: Hongze Zhang
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.20.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> A Java-oriented field type loses its nullable constraint after calling method
> {{org.apache.calcite.jdbc.JavaTypeFactoryImpl#toSql(org.apache.calcite.rel.type.RelDataType)}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-3029) Java-oriented field type is wrongly forced to be NOT NULL after being converted to SQL-oriented
[
https://issues.apache.org/jira/browse/CALCITE-3029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16827905#comment-16827905
]
Hongze Zhang commented on CALCITE-3029:
---
It seems that the issue is introduced by CALCITE-2464 fix.
[~rubenql], [~zabetak], Could you please help me review
https://github.com/apache/calcite/pull/1187? Thank you very much.
> Java-oriented field type is wrongly forced to be NOT NULL after being
> converted to SQL-oriented
> ---
>
> Key: CALCITE-3029
> URL: https://issues.apache.org/jira/browse/CALCITE-3029
> Project: Calcite
> Issue Type: Bug
>Affects Versions: 1.19.0
>Reporter: Hongze Zhang
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.20.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> A Java-oriented field type loses its nullable constraint after calling method
> {{org.apache.calcite.jdbc.JavaTypeFactoryImpl#toSql(org.apache.calcite.rel.type.RelDataType)}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
