You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2022/07/06 05:46:14 UTC
[GitHub] [calcite] joshuamaurice opened a new pull request, #2852: Fix ColumnMetaData.Rep for nested arrays
joshuamaurice opened a new pull request, #2852:
URL: https://github.com/apache/calcite/pull/2852
For array of array of int (nested arrays), the Rep of one of the arrays is PRIMITIVE_INT, not MULTISET. This breaks when calling ResultSet.getObject, failing in the array-accessor with the wrong Rep.
I am very new to Apache Calcite. Based on my preliminary testing, this appears to be the correct change. However, I don't have any proper automated tests for this change to add as part of this pull request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] joshuamaurice commented on pull request #2852: Fix ColumnMetaData.Rep for nested arrays
Posted by GitBox <gi...@apache.org>.
joshuamaurice commented on PR #2852:
URL: https://github.com/apache/calcite/pull/2852#issuecomment-1175866878
Hi Alessandro Solimando,
Ok, thanks. Will do
On Wed, Jul 6, 2022 at 12:09 AM Alessandro Solimando <
***@***.***> wrote:
> Hi @joshuamaurice <https://github.com/joshuamaurice>, I don't know why
> the PR was closed without any comments, this should not happen.
>
> I haven't looked at the fix, but for each non-trivial change (e.g., fixing
> a typo) we need a Jira ticket, please refer to the contributor's guideline
> <https://calcite.apache.org/develop/#contributing> for all the details.
>
> I read you haven't any tests for confirming the limitation (and that your
> fix is correct), this won't allow to merge the PR. Please file a ticket
> (following the guidelines for the title and description), try to first
> derive such a test (you might for instance use coverage tools like in
> IntelliJ with "line counts" to see which UTs cover the code you are willing
> to fix, and try to derive a test from there, if you fail describe what you
> have tried in the ticket and we will try to suggest where to start.
>
> Welcome to the community!
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/calcite/pull/2852#issuecomment-1175860897>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ADGXHS6YCHTDRLW74FI4VH3VSUWJDANCNFSM52YNMWKA>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] joshuamaurice commented on pull request #2852: Fix ColumnMetaData.Rep for nested arrays
Posted by GitBox <gi...@apache.org>.
joshuamaurice commented on PR #2852:
URL: https://github.com/apache/calcite/pull/2852#issuecomment-1175867217
Also, already made the bug.
https://issues.apache.org/jira/browse/CALCITE-5204
On Wed, Jul 6, 2022 at 12:16 AM Joshua Maurice ***@***.***>
wrote:
> Hi Alessandro Solimando,
> Ok, thanks. Will do
>
>
> On Wed, Jul 6, 2022 at 12:09 AM Alessandro Solimando <
> ***@***.***> wrote:
>
>> Hi @joshuamaurice <https://github.com/joshuamaurice>, I don't know why
>> the PR was closed without any comments, this should not happen.
>>
>> I haven't looked at the fix, but for each non-trivial change (e.g.,
>> fixing a typo) we need a Jira ticket, please refer to the contributor's
>> guideline <https://calcite.apache.org/develop/#contributing> for all the
>> details.
>>
>> I read you haven't any tests for confirming the limitation (and that your
>> fix is correct), this won't allow to merge the PR. Please file a ticket
>> (following the guidelines for the title and description), try to first
>> derive such a test (you might for instance use coverage tools like in
>> IntelliJ with "line counts" to see which UTs cover the code you are willing
>> to fix, and try to derive a test from there, if you fail describe what you
>> have tried in the ticket and we will try to suggest where to start.
>>
>> Welcome to the community!
>>
>> —
>> Reply to this email directly, view it on GitHub
>> <https://github.com/apache/calcite/pull/2852#issuecomment-1175860897>,
>> or unsubscribe
>> <https://github.com/notifications/unsubscribe-auth/ADGXHS6YCHTDRLW74FI4VH3VSUWJDANCNFSM52YNMWKA>
>> .
>> You are receiving this because you were mentioned.Message ID:
>> ***@***.***>
>>
>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] joshuamaurice commented on pull request #2852: Fix ColumnMetaData.Rep for nested arrays
Posted by GitBox <gi...@apache.org>.
joshuamaurice commented on PR #2852:
URL: https://github.com/apache/calcite/pull/2852#issuecomment-1175828625
Hi. I see the pull requested was closed (denied). I'm sorry. I'm new to Apache Calcite and github, and I don't know the proper etiquette. Should I file a bug in github first for this issue? Or is my fix incorrect?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] asolimando commented on pull request #2852: Fix ColumnMetaData.Rep for nested arrays
Posted by GitBox <gi...@apache.org>.
asolimando commented on PR #2852:
URL: https://github.com/apache/calcite/pull/2852#issuecomment-1175860897
Hi @joshuamaurice, I don't know why the PR was closed without any comments, this should not happen.
I haven't looked at the fix, but for each non-trivial change (e.g., fixing a typo) we need a Jira ticket, please refer to the [contributor's guideline](https://calcite.apache.org/develop/#contributing) for all the details.
I read you haven't any tests for confirming the limitation (and that your fix is correct), this won't allow to merge the PR. Please file a ticket (following the guidelines for the title and description), try to first derive such a test (you might for instance use coverage tools like in IntelliJ with "line counts" to see which UTs cover the code you are willing to fix, and try to derive a test from there, if you fail describe what you have tried in the ticket and we will try to suggest where to start.
Welcome to the community!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org