c2c: allow user to pass start time as a decimal to crdb_internal.fingerprint() #104219
Conversation
|
This change is |
msbutler
force-pushed
the
butler-fingerprint-input
branch
2 times, most recently
from
40f9e68 to
90287ed
Compare
msbutler
requested review from
srosenberg and
smg260
and removed request for
a team
| }, | ||
| Info: "This function is used only by CockroachDB's developers for testing purposes.", | ||
| Volatility: volatility.Stable, | ||
| }, | ||
| tree.Overload{ | ||
| Types: tree.ParamTypes{ | ||
| {Name: "span", Typ: types.BytesArray}, |
There was a problem hiding this comment.
do we still need the TimestampTZ overload? or can we just replace it with the more accurate decimal overload that you added?
There was a problem hiding this comment.
I kept the TimestampTZ overload because it's userfriendly and bc i'm lazy and didn't want to refactor all the tests that already use it :D.
| var fingerprint int64 | ||
| query := fmt.Sprintf(`SELECT * FROM crdb_internal.fingerprint(ARRAY[$1::BYTES, $2::BYTES], $3, $4) AS OF SYSTEM TIME %s`, decimal.String()) | ||
| require.NoError(t, sqlDB.QueryRow(query, roachpb.Key(startKey), roachpb.Key(endKey), startTime.GoTime(), allRevisions).Scan(&fingerprint)) | ||
| query := fmt.Sprintf(`SELECT * FROM crdb_internal.fingerprint(ARRAY[$1::BYTES, $2::BYTES],$3::DECIMAL, $4) AS OF SYSTEM TIME '%s'`, aost) |
There was a problem hiding this comment.
so this test is for the decimal overload, do we have a test for the TimestampTZ one?
There was a problem hiding this comment.
yeah, there's a test below this one in this package and plenty of other unit tests in other packages use the timestamp overload.
…erprint() Previously, the user could only pass the startTime variable to crdb_internal.fingerprint() as a TimestampTZ, which only has microsecond precision. To enable nanosecond precision, this patch allows the user to pass the startTime as a decimal. This patch is consistent with the user's ability to pass and AOST timestamp as a decimal as well. Informs cockroachdb#103072 Release note: None
msbutler
force-pushed
the
butler-fingerprint-input
branch
from
90287ed to
bb45084
Compare
|
TFTR!! bors r=lidorcarmel |
|
This PR was included in a batch that successfully built, but then failed to merge into master. It will not be retried. Additional information: {"message":"Changes must be made through a pull request.","documentation_url":"https://docs.github.com/articles/about-protected-branches"} |
|
bors r+ single on |
|
Build succeeded: |
|
blathers backport 23.1 |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from bb45084 to blathers/backport-release-23.1-104219: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 23.1 failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Previously, the user could only pass the startTime variable to crdb_internal.fingerprint() as a TimestampTZ, which only has microsecond precision. To enable nanosecond precision, this patch allows the user to pass the startTime as a decimal. This patch is consistent with the user's ability to pass and AOST timestamp as a decimal as well.
Informs #103072
Release note: None