-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Update ClickBench queries to avoid to_timestamp_seconds #15475
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Since we're only ordering by this it shouldn't matter that we order by an integer instead of a proper timestamp, ordering is equivalent.
Should we update the same query in the clickbench repo as well? |
Yes, and we might rerun the queries as well (as |
Thank you very much for your reply, but I have a question: why can't the following |
I'm agreeing with you! I think this change is good 😄 |
I have also made changes to the query in ClickBench: clickbench-pr |
@Dandandan do you think we can merge this here? |
Thank you @acking-you |
Which issue does this PR close?
to_timestamp_seconds
#15465 .Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?