Skip to content

workload: fix pgx error cast in kv95 and schemachange#69212

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
rafiss:fix-pgconn-err-cast
Aug 23, 2021
Merged

workload: fix pgx error cast in kv95 and schemachange#69212
craig[bot] merged 1 commit intocockroachdb:masterfrom
rafiss:fix-pgconn-err-cast

Conversation

@rafiss
Copy link
Copy Markdown
Collaborator

@rafiss rafiss commented Aug 21, 2021 •
edited
Loading

fixes #69189
#69100 is failing but i'm not sure if this is the cause. (cc @ajwerner)

This was done incorrectly after the recent upgrade to pgx4.
pgconn.PgError does not implement error, but *pgconn.PgError
does.

Release justification: test only change
Release note: None

@rafiss
workload: fix pgx error cast in kv95 and schemachange
70f42e0 
This was done incorrectly after the recent upgrade to pgx4.
`pgconn.PgError` does not implement `error`, but `*pgconn.PgError`
does.

Release note: None
@rafiss rafiss requested review from a team and stevendanna and removed request for a team August 21, 2021 05:41
@cockroach-teamcity
Copy link
Copy Markdown
Member

cockroach-teamcity commented Aug 21, 2021

This change is Reviewable

RichardJCai
RichardJCai approved these changes Aug 23, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@RichardJCai RichardJCai left a comment

Choose a reason for hiding this comment

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

LGTM

But wondering what the difference is between doing
new(pgconn.PgError) vs &pgconn.PgError{}

We do &pgconn.PgError{} here https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/conn_executor_test.go#L792

@ajwerner
Copy link
Copy Markdown
Contributor

ajwerner commented Aug 23, 2021

But wondering what the difference is between doing
new(pgconn.PgError) vs &pgconn.PgError{}

We do &pgconn.PgError{} here https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/conn_executor_test.go#L792

Semantically they are equivalent. There's an ancient thread about it here: https://groups.google.com/g/golang-nuts/c/GDXFDJgKKSs.

@rafiss
Copy link
Copy Markdown
Collaborator Author

rafiss commented Aug 23, 2021

new(pgconn.PgError) and &pgconn.PgError{} are actually the same thing

i just changed to new(pgconn.PgError) since it made the code look slightly less confusing, since errors.As needs a & operator right after initializing the variable. i missed the one you pointed out, i'll change it too

@rafiss
Copy link
Copy Markdown
Collaborator Author

rafiss commented Aug 23, 2021

nvm i didn't miss it -- that was one of the lines i changed. tftr!

bors r=RichardJCai

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 23, 2021

Build succeeded:

@craig craig bot merged commit afaa938 into cockroachdb:master Aug 23, 2021
@rafiss rafiss deleted the fix-pgconn-err-cast branch August 24, 2021 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stevendanna stevendanna Awaiting requested review from stevendanna

1 more reviewer

@RichardJCai RichardJCai RichardJCai approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

roachtest: kv95limited-spans/enc=false/nodes=1/cpu=32/splt=0/seq/no-load-splitting failed

4 participants

@rafiss @cockroach-teamcity @ajwerner @RichardJCai