-
Notifications
You must be signed in to change notification settings - Fork 236
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CALCITE-4600] ClassCastException for arrays with date/time/timestamp elements #154
Conversation
…date/time/timestamp array accessors
…omNumberAccessor#getNumber
Thanks for doing this. I just ran your top commit and I got a couple of failures:
Let's figure this out, and I'll merge. |
I can't reproduce the failure locally on MacOSX (I have tried few JVMs), and it seems to work fine in CI. I made sure to skip test cache which sometimes fools me by running with: Could you share the relevant details of your environment so I can try to reproduce the issue? Could you try if the failure happens with the original PR as well? Thanks! |
I created a PR to this branch with a fix for a problem in different timezone |
Thanks @snuyanzin, I have tested it locally with different timezones and it works fine. |
I'll merge shortly. |
Rebasing https://github.com/apache/calcite-avatica/pulls on master and fixing conflicts.
The original PR includes commits for both [CALCITE-4600] and [CALCITE-4602].
While fixing conflicts I found out that merged PRs for [CALCITE-4536] and [CALCITE-4757] added tests but did not enable them, so I have added two extra commits to enable them (I think it's needed since they are already merged and that code is untested at the moment).
PS: whenever a new data type is added to
@Override public ExecuteResult prepareAndExecute(
, it must be paired with an addition underpublic static Collection<AccessorTestHelper> data()
, otherwise it won't be called.PPS: I suggested to the committer to squash the first two commits starting with
[CALCITE-4600]
(I did not do it because it was like this in the original PR, it's easier to check what I changed) and leave the rest as separate commits for clarity.