-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
make sure to send down timestamp queries to vttablet #16819
Conversation
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
e93673c
to
5030dd0
Compare
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.
Is it not incorrect to route these queries to a random shard? If someone runs:
START TRANSACTION;
SELECT * FROM `users` WHERE `id` = 1;
SELECT NOW();
COMMIT;
Where the users table is sharded on id, one might expect that the SELECT NOW()
is run on the same machine as the SELECT FROM users
, so that the transaction stays single-shard and so that NOW() returns the time at the start of the transaction, which is how unsharded MySQL works.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16819 +/- ##
==========================================
+ Coverage 68.98% 69.50% +0.52%
==========================================
Files 1562 1568 +6
Lines 200690 202480 +1790
==========================================
+ Hits 138449 140743 +2294
+ Misses 62241 61737 -504 ☔ View full report in Codecov by Sentry. |
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.
Let's add a e2e test for it to validate the fix.
Good point. I'll update the PR and try again. |
Signed-off-by: Andres Taylor <[email protected]>
5030dd0
to
ca83068
Compare
Replaced by #16824 |
Description
The time zone system setting was not being fetched correctly, which lead to it being ignored when evaluating
now()
.Related Issue(s)
Fixes #16820
Checklist