Skip to content

[CALCITE-7494] Avatica conversion to string of TIMESTAMP WITH TIME ZONE does not include time zone - #314

Open
mihaibudiu wants to merge 1 commit into
apache:mainfrom
mihaibudiu:issue7494
Open

[CALCITE-7494] Avatica conversion to string of TIMESTAMP WITH TIME ZONE does not include time zone#314
mihaibudiu wants to merge 1 commit into
apache:mainfrom
mihaibudiu:issue7494

Conversation

@mihaibudiu

Copy link
Copy Markdown
Contributor

No description provided.

…NE does not include time zone

Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
@mihaibudiu

Copy link
Copy Markdown
Contributor Author

This affects the Calcite output, causing CI to fail.

@mihaibudiu

Copy link
Copy Markdown
Contributor Author

I guess I should first submit a helper PR in Calcite to disable the failing tests, and after merging this we can re-enable them.

@mihaibudiu

Copy link
Copy Markdown
Contributor Author

Whoever is reviewing this, please also check the failing CI test output, since it shows what the Calcite results will be in the future if we merge this.

@xiedeyantu

xiedeyantu commented Sep 7, 2026

Copy link
Copy Markdown
Member

As I understand it, the issue is simply that Avatica was losing the "UTC" designation, so the goal of this PR (including the tests) is to verify that the fix preserves that designation. Therefore, there is no need to verify the correctness of the time zone handling itself (such as in the example below)—is that correct?

select TIMESTAMP WITH TIME ZONE '2020-01-01 00:00:00 America/New_York';
+---------------------+
| EXPR$0              |
+---------------------+
| 2020-01-01 05:00:00 |
+---------------------+

@mihaibudiu

Copy link
Copy Markdown
Contributor Author

Yes, this is right. This will need to be merged in 3 steps: disable calcite tests, merge this, enable calcite tests

@F21

F21 commented Sep 7, 2026

Copy link
Copy Markdown
Member

@mihaibudiu I can see that @xiedeyantu has approved this PR. Can you go ahead with the required merges to get this in?

@mihaibudiu

Copy link
Copy Markdown
Contributor Author

@F21 once we merge apache/calcite#5247 we can hopefully merge this as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants