vitessio / vitess

Vitess is a database clustering system for horizontal scaling of MySQL.
http://vitess.io
Apache License 2.0
18.67k stars 2.1k forks source link

make sure to send down timestamp queries to vttablet #16819

Closed systay closed 1 month ago

systay commented 1 month ago

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

vitess-bot[bot] commented 1 month ago

Review Checklist

Hello reviewers! :wave: Please follow this checklist when reviewing this Pull Request.

General

Tests

Documentation

New flags

If a workflow is added or modified:

Backward compatibility

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 85.71429% with 3 lines in your changes missing coverage. Please review.

Project coverage is 69.50%. Comparing base (538dd4c) to head (ca83068). Report is 93 commits behind head on main.

Files with missing lines Patch % Lines
go/vt/vtgate/vcursor_impl.go 83.33% 3 Missing :warning:
Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

systay commented 1 month ago

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.

Good point. I'll update the PR and try again.

systay commented 1 month ago

Replaced by #16824