[jira] [Commented] (CALCITE-1442) SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault() returns the wrong field
[
https://issues.apache.org/jira/browse/CALCITE-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15577052#comment-15577052
]
Laurent Goujon commented on CALCITE-1442:
-
Updated my pull request with a new test case in SqlValidatorTest:
{code:java}
checkExpType(
"CASE 1 WHEN 1 THEN INTERVAL '12 3:4:5.6' DAY TO SECOND(6) WHEN 2 THEN
INTERVAL '12 3:4:5.6' DAY TO SECOND(9) END",
"INTERVAL DAY TO SECOND(9)");
{code}
With the current code, returned type is {{INTERVAL DAY TO SECOND(6)}}.
> SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault() returns
> the wrong field
> ---
>
> Key: CALCITE-1442
> URL: https://issues.apache.org/jira/browse/CALCITE-1442
> Project: Calcite
> Issue Type: Bug
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Minor
>
> Unless I'm wrong, I believe
> {{SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault()}}
> returns the wrong field:
> {code:java}
> public int getFractionalSecondPrecision(RelDataTypeSystem typeSystem) {
> if (fractionalSecondPrecision == RelDataType.PRECISION_NOT_SPECIFIED) {
> return typeName().getDefaultScale();
> } else {
> return fractionalSecondPrecision;
> }
> }
> public int getFractionalSecondPrecisionPreservingDefault() {
> if (useDefaultFractionalSecondPrecision()) {
> return RelDataType.PRECISION_NOT_SPECIFIED;
> } else {
> return startPrecision;
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CALCITE-1442) SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault() returns the wrong field
[
https://issues.apache.org/jira/browse/CALCITE-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15577033#comment-15577033
]
Laurent Goujon commented on CALCITE-1442:
-
if you mean "debugging what the value was not the one I was expecting in
Dremio, and clicking on call hierarchy on my IDE to see where this function was
used", yes I deeply studied the code :) and the code is present from the early
days I guess (if I'm correct, introduced in commit
b0dab683059fa1163dc95cb9ea7540ad6a4968ab)
> SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault() returns
> the wrong field
> ---
>
> Key: CALCITE-1442
> URL: https://issues.apache.org/jira/browse/CALCITE-1442
> Project: Calcite
> Issue Type: Bug
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Minor
>
> Unless I'm wrong, I believe
> {{SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault()}}
> returns the wrong field:
> {code:java}
> public int getFractionalSecondPrecision(RelDataTypeSystem typeSystem) {
> if (fractionalSecondPrecision == RelDataType.PRECISION_NOT_SPECIFIED) {
> return typeName().getDefaultScale();
> } else {
> return fractionalSecondPrecision;
> }
> }
> public int getFractionalSecondPrecisionPreservingDefault() {
> if (useDefaultFractionalSecondPrecision()) {
> return RelDataType.PRECISION_NOT_SPECIFIED;
> } else {
> return startPrecision;
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CALCITE-1442) SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault() returns the wrong field
[
https://issues.apache.org/jira/browse/CALCITE-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15576994#comment-15576994
]
Julian Hyde commented on CALCITE-1442:
--
I'm not surprised at all that there's no existing test. Having spent the day
studying this code, you probably know more about it than anyone else in the
world, so you are in a good position to write a few tests. :)
> SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault() returns
> the wrong field
> ---
>
> Key: CALCITE-1442
> URL: https://issues.apache.org/jira/browse/CALCITE-1442
> Project: Calcite
> Issue Type: Bug
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Minor
>
> Unless I'm wrong, I believe
> {{SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault()}}
> returns the wrong field:
> {code:java}
> public int getFractionalSecondPrecision(RelDataTypeSystem typeSystem) {
> if (fractionalSecondPrecision == RelDataType.PRECISION_NOT_SPECIFIED) {
> return typeName().getDefaultScale();
> } else {
> return fractionalSecondPrecision;
> }
> }
> public int getFractionalSecondPrecisionPreservingDefault() {
> if (useDefaultFractionalSecondPrecision()) {
> return RelDataType.PRECISION_NOT_SPECIFIED;
> } else {
> return startPrecision;
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CALCITE-1442) SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault() returns the wrong field
[
https://issues.apache.org/jira/browse/CALCITE-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15576961#comment-15576961
]
Laurent Goujon commented on CALCITE-1442:
-
The issue surfaced when testing a change in Drill in the view system, where the
schema is serialized/deserialized and during deserialization the types are not
matching anymore:
https://github.com/apache/drill/blob/master/exec/java-exec/src/main/java/org/apache/drill/exec/dotdrill/View.java#L185
Maybe this method is doing the right thing, but why then:
- the name of the method starts with {{getFractionalSecondPrecision}}?
- {{getFractionalSecondPrecision(RelDataTypeSystem)}} returns
{{fractionalSecondPrecision}}, and not {{startPrecision}}
- what's the difference then with {{getStartPrecisionPreservingDefault()}}?
The two places where I saw this method used is in Drill, and in static method
{{combineFractionalSecondPrecisionPreservingDefault}}, where this method is
used to combine {{IntervalSqlType}} instances, by storing the result into the
field fracPrec.
So from the look of it, everything seems to believe this method is supposed to
return the fractional second precision.
I'm willing to add a test case, but I'm not sure which kind (also calcite
didn't fail with my patch so I guess it means there's no existing test
depending on that behaviour)?
> SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault() returns
> the wrong field
> ---
>
> Key: CALCITE-1442
> URL: https://issues.apache.org/jira/browse/CALCITE-1442
> Project: Calcite
> Issue Type: Bug
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Minor
>
> Unless I'm wrong, I believe
> {{SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault()}}
> returns the wrong field:
> {code:java}
> public int getFractionalSecondPrecision(RelDataTypeSystem typeSystem) {
> if (fractionalSecondPrecision == RelDataType.PRECISION_NOT_SPECIFIED) {
> return typeName().getDefaultScale();
> } else {
> return fractionalSecondPrecision;
> }
> }
> public int getFractionalSecondPrecisionPreservingDefault() {
> if (useDefaultFractionalSecondPrecision()) {
> return RelDataType.PRECISION_NOT_SPECIFIED;
> } else {
> return startPrecision;
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CALCITE-1442) SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault() returns the wrong field
[
https://issues.apache.org/jira/browse/CALCITE-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15576924#comment-15576924
]
Julian Hyde commented on CALCITE-1442:
--
I'm not sure. I tinkered with that code a while ago, and came to the conclusion
that while it looks wrong, it's actually correct. Can you provide a test case
that proves there's a bug here?
> SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault() returns
> the wrong field
> ---
>
> Key: CALCITE-1442
> URL: https://issues.apache.org/jira/browse/CALCITE-1442
> Project: Calcite
> Issue Type: Bug
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Minor
>
> Unless I'm wrong, I believe
> {{SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault()}}
> returns the wrong field:
> {code:java}
> public int getFractionalSecondPrecision(RelDataTypeSystem typeSystem) {
> if (fractionalSecondPrecision == RelDataType.PRECISION_NOT_SPECIFIED) {
> return typeName().getDefaultScale();
> } else {
> return fractionalSecondPrecision;
> }
> }
> public int getFractionalSecondPrecisionPreservingDefault() {
> if (useDefaultFractionalSecondPrecision()) {
> return RelDataType.PRECISION_NOT_SPECIFIED;
> } else {
> return startPrecision;
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CALCITE-1442) SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault() returns the wrong field
[
https://issues.apache.org/jira/browse/CALCITE-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15576833#comment-15576833
]
Laurent Goujon commented on CALCITE-1442:
-
Proposed change: https://github.com/apache/calcite/pull/309
> SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault() returns
> the wrong field
> ---
>
> Key: CALCITE-1442
> URL: https://issues.apache.org/jira/browse/CALCITE-1442
> Project: Calcite
> Issue Type: Bug
>Reporter: Laurent Goujon
>Assignee: Julian Hyde
>Priority: Minor
>
> Unless I'm wrong, I believe
> {{SqlIntervalQualifer#getFractionalSecondPrecisionPreservingDefault()}}
> returns the wrong field:
> {code:java}
> public int getFractionalSecondPrecision(RelDataTypeSystem typeSystem) {
> if (fractionalSecondPrecision == RelDataType.PRECISION_NOT_SPECIFIED) {
> return typeName().getDefaultScale();
> } else {
> return fractionalSecondPrecision;
> }
> }
> public int getFractionalSecondPrecisionPreservingDefault() {
> if (useDefaultFractionalSecondPrecision()) {
> return RelDataType.PRECISION_NOT_SPECIFIED;
> } else {
> return startPrecision;
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
