[jira] [Commented] (CALCITE-4510) Weird digests for literals with some user defined types
[
https://issues.apache.org/jira/browse/CALCITE-4510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17291483#comment-17291483
]
Ruben Q L commented on CALCITE-4510:
Thanks [~fan_li_ya].
I have a final remark, I think the commit message (and this Jira's title) could
be a bit more precise and descriptive. I'd suggest "RexLiteral can produce
wrong digest for some user defined types", or something like that. What do you
think?
> Weird digests for literals with some user defined types
> ---
>
> Key: CALCITE-4510
> URL: https://issues.apache.org/jira/browse/CALCITE-4510
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Liya Fan
>Assignee: Liya Fan
>Priority: Minor
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> We find weird literals for some user defined non-nullable types. Some
> investigation shows that the problem lies in the {{RexLiteral#toJavaString}}
> method.
> In particular, it checks the type string suffix with an 8-character string:
> {noformat}
> if (!fullTypeString.endsWith("NOT NULL")) {
> {noformat}
> However, it trims the last 9 characters from the end of the string:
> {noformat}
> sb.append(fullTypeString, 0, fullTypeString.length() - 9);
> {noformat}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4510) Weird digests for literals with some user defined types
[
https://issues.apache.org/jira/browse/CALCITE-4510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17291357#comment-17291357
]
Liya Fan commented on CALCITE-4510:
---
[~rubenql] Sounds reasonable. Thanks a lot for your effort and kind reminder.
Sorry I should have done the investigation myself.
> Weird digests for literals with some user defined types
> ---
>
> Key: CALCITE-4510
> URL: https://issues.apache.org/jira/browse/CALCITE-4510
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Liya Fan
>Assignee: Liya Fan
>Priority: Minor
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> We find weird literals for some user defined non-nullable types. Some
> investigation shows that the problem lies in the {{RexLiteral#toJavaString}}
> method.
> In particular, it checks the type string suffix with an 8-character string:
> {noformat}
> if (!fullTypeString.endsWith("NOT NULL")) {
> {noformat}
> However, it trims the last 9 characters from the end of the string:
> {noformat}
> sb.append(fullTypeString, 0, fullTypeString.length() - 9);
> {noformat}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4510) Weird digests for literals with some user defined types
[
https://issues.apache.org/jira/browse/CALCITE-4510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17290783#comment-17290783
]
Ruben Q L commented on CALCITE-4510:
Thanks for the explanation, [~fan_li_ya].
I agree removing this hard-coded " NOT NULL" that is all around the place and
centralize it in a single constant is a good idea (plus it solves your problem
with user-defined type systems).
Searching the code, it seems the PR could go a bit further and do the
replacement in other classes too:
- RelDatatypeImpl (already covered in the initial PR)
- RexLiteral (already covered in the initial PR)
- SqlTypeUtil#equalSansNullability
- RelOptUtil#TypeDumper#accept (x2)
And even in some tests classes:
- SqlTests#getTypeString
- SqlOperatorBaseTest (x4): checkCastToApproxOkay, checkCastToStringOkay,
checkCastToScalarOkay, assertSubFunReturns
- CalciteAssert#typeString
- RexProgramTestBase#assertNode
- RexProgramTest#assertTypeAndToString
WDYT?
> Weird digests for literals with some user defined types
> ---
>
> Key: CALCITE-4510
> URL: https://issues.apache.org/jira/browse/CALCITE-4510
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Liya Fan
>Assignee: Liya Fan
>Priority: Minor
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> We find weird literals for some user defined non-nullable types. Some
> investigation shows that the problem lies in the {{RexLiteral#toJavaString}}
> method.
> In particular, it checks the type string suffix with an 8-character string:
> {noformat}
> if (!fullTypeString.endsWith("NOT NULL")) {
> {noformat}
> However, it trims the last 9 characters from the end of the string:
> {noformat}
> sb.append(fullTypeString, 0, fullTypeString.length() - 9);
> {noformat}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4510) Weird digests for literals with some user defined types
[
https://issues.apache.org/jira/browse/CALCITE-4510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17290649#comment-17290649
]
Liya Fan commented on CALCITE-4510:
---
[~rubenql] Thanks for your feedback.
It is difficult to give a proper unit test, as it works well for the existing
type system (as you have indicated).
The problem may occur for some user defined types, for which there is not a
space before "NOT NULL".
It could be argued that the digest of the user defined type has some flaws that
need to be fixed. That is true.
However, I think the Calcite implementation can also be improved.
> Weird digests for literals with some user defined types
> ---
>
> Key: CALCITE-4510
> URL: https://issues.apache.org/jira/browse/CALCITE-4510
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Liya Fan
>Assignee: Liya Fan
>Priority: Minor
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> We find weird literals for some user defined non-nullable types. Some
> investigation shows that the problem lies in the {{RexLiteral#toJavaString}}
> method.
> In particular, it checks the type string suffix with an 8-character string:
> {noformat}
> if (!fullTypeString.endsWith("NOT NULL")) {
> {noformat}
> However, it trims the last 9 characters from the end of the string:
> {noformat}
> sb.append(fullTypeString, 0, fullTypeString.length() - 9);
> {noformat}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4510) Weird digests for literals with some user defined types
[
https://issues.apache.org/jira/browse/CALCITE-4510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17289785#comment-17289785
]
Ruben Q L commented on CALCITE-4510:
Could we see the issue in a unit test?
I agree the current code is not very elegant, but it would seem it "checks the
type string suffix with an 8-character string", i.e. it checks {{endsWith("NOT
NULL")}}
And then "it trims the last 9 characters from the end of the string", just
because it also trims the white-space that precedes that string: " NOT NULL";
e.g. for a digest "XYZ NOT NULL" we end up with "XYZ"; which seems fine. Is
there an issue in that? Am I missing something?
> Weird digests for literals with some user defined types
> ---
>
> Key: CALCITE-4510
> URL: https://issues.apache.org/jira/browse/CALCITE-4510
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: Liya Fan
>Assignee: Liya Fan
>Priority: Minor
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> We find weird literals for some user defined non-nullable types. Some
> investigation shows that the problem lies in the {{RexLiteral#toJavaString}}
> method.
> In particular, it checks the type string suffix with an 8-character string:
> {noformat}
> if (!fullTypeString.endsWith("NOT NULL")) {
> {noformat}
> However, it trims the last 9 characters from the end of the string:
> {noformat}
> sb.append(fullTypeString, 0, fullTypeString.length() - 9);
> {noformat}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
