Skip to content

Disable timestamp#23191

Closed
amitkdutta wants to merge 1 commit intoprestodb:masterfrom
amitkdutta:tz_disable2
Closed

Disable timestamp#23191
amitkdutta wants to merge 1 commit intoprestodb:masterfrom
amitkdutta:tz_disable2

Conversation

@amitkdutta
Copy link
Contributor

No description provided.

@amitkdutta amitkdutta requested a review from a team as a code owner July 12, 2024 03:35
@amitkdutta amitkdutta requested a review from presto-oss July 12, 2024 03:35
@amitkdutta amitkdutta marked this pull request as draft July 12, 2024 03:35
@amitkdutta
Copy link
Contributor Author

One thing I am not clear is why the following queries are failing

https://app.circleci.com/pipelines/github/prestodb/presto/17783/workflows/0ac21bfd-fa6a-4bdc-b5f1-4c9212bd4a78/jobs/70265?invite=true#step-105-147520_145

SELECT case linenumber % 10 when orderkey % 3 then orderkey + 1 when 2 then orderkey + 2 else 0 end FROM lineitem

https://app.circleci.com/pipelines/github/prestodb/presto/17783/workflows/0ac21bfd-fa6a-4bdc-b5f1-4c9212bd4a78/jobs/70265?invite=true#step-105-47684_41

SELECT reduce_agg(orderkey, 0, (x, y) -> x + y, (x, y) -> x + y) FROM orders

@aditi-pandit
Copy link
Contributor

@amitkdutta
Copy link
Contributor Author

@amitkdutta : This might have come from a Plugin parametric type https://github.com/prestodb/presto/blob/master/presto-main/src/main/java/com/facebook/presto/server/PluginManager.java#L241.

Do you have any such setup ?

Not sure about the Parametric type. Will be great if you can suggest what check we need to add to make the query pass. CC: @tdcmeehan

@gggrace14
Copy link
Contributor

@tdcmeehan Hi Tim, can you help confirm that our solution is in the right direction? This PR is trying to solve the issue of #23179 at the coordinator. Then we will continue to get the e2e test to pass through

@feilong-liu
Copy link
Contributor

The failure here is due to functions defined here for timestamp with timezone type https://github.com/prestodb/presto/blob/master/presto-main/src/main/java/com/facebook/presto/type/DateTimeOperators.java

@feilong-liu
Copy link
Contributor

If the goal of this PR is to fail the query with this timestamp with timezone type early, simply deleting this type may be too much work, as there can be many references of this type. A simple way may be to add a plan validator which only runs for Prestissimo and fail query if it has this type.

@amitkdutta
Copy link
Contributor Author

Done in #23200

@amitkdutta amitkdutta closed this Jul 13, 2024
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.

4 participants