Skip to content
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

sqlccl: test prepared statement in session migration test #108305

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

rafiss
Copy link
Collaborator

@rafiss rafiss commented Aug 7, 2023

There are other tests in pkg/sql/testdata/session_migration but the one in this PR is a full integration test.

informs https://github.com/cockroachlabs/support/issues/2510
Release note: None

@rafiss rafiss requested a review from a team as a code owner August 7, 2023 17:29
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: Thanks! Any luck with UDTs on a reproduction?

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @ecwall and @jaylim-crl)

@rafiss rafiss force-pushed the session-migration-prep-stmt branch from 76ad4c7 to 1f2738c Compare August 7, 2023 19:23
@rafiss
Copy link
Collaborator Author

rafiss commented Aug 7, 2023

Yeah, UDTs worked in the simple test I made. I updated the PR now.

@rafiss rafiss force-pushed the session-migration-prep-stmt branch from 1f2738c to 7ebca5e Compare August 7, 2023 19:24
@@ -20,6 +20,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/jackc/pgx/v4"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use v5 or is v4 required for this test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@rafiss rafiss force-pushed the session-migration-prep-stmt branch from 7ebca5e to c0867be Compare August 8, 2023 04:46
@rafiss
Copy link
Collaborator Author

rafiss commented Aug 8, 2023

tftr

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 8, 2023

Build failed:

@rafiss rafiss force-pushed the session-migration-prep-stmt branch from c0867be to 70adada Compare August 8, 2023 15:09
@rafiss
Copy link
Collaborator Author

rafiss commented Aug 8, 2023

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 8, 2023

This PR was included in a batch that timed out, it will be automatically retried

@craig
Copy link
Contributor

craig bot commented Aug 8, 2023

Build succeeded:

@craig craig bot merged commit 3ef6592 into cockroachdb:master Aug 8, 2023
@rafiss rafiss deleted the session-migration-prep-stmt branch August 9, 2023 03:55
craig bot pushed a commit that referenced this pull request Aug 10, 2023
108179: concurrency: use lock modes to detect conflicts during optimistic eval r=nvanbenschoten a=arulajmani

This patch switches over conflict resolution performed by optimistic evaluation to use lock modes instead of ad-hoc logic. As a result of this, optimistic evaluation is able to handle shared locks. We add a test to show this.

Closes #108142

Release note: None

108503: sql: do not evaluate AOST timestamp in session migrations r=rafiss a=rafiss

fixes https://github.com/cockroachlabs/support/issues/2510
refs #108305
Release note (bug fix): Fixed a bug where a session migration performed by SHOW TRANSFER STATE would not handle prepared statements that used the AS OF SYSTEM TIME clause. Users who encountered this bug would see errors such as `expected 1 or 0 for number of format codes, got N`. This bug was present since v22.2.0.

108523: roachtest: Reset job load attempt when loading cdc job r=miretskiy a=miretskiy

Fixes #108433

Release note: None

Co-authored-by: Arul Ajmani <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Yevgeniy Miretskiy <[email protected]>
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