sql: errors from ResolveOIDFromOID are ignored

[fqazi]

When we hit transaction retry errors the following code paths can end up swallowing errors, which can lead to other issues:

cockroach/pkg/sql/sem/eval/cast.go

Line 959 in cc5bde5

if err != nil {

cockroach/pkg/sql/sem/eval/cast.go

Line 958 in cc5bde5

dOid, err := res.ResolveOIDFromOID(ctx, t, tree.NewDOid(o))

We ran into this scenario in the randomized schema changer workload where OID casts combined with transaction retry errors could lead to failures.

Jira issue: CRDB-17664
Epic CRDB-14049

[fqazi]

Similar problem at :

cockroach/pkg/sql/sem/eval/cast.go

Line 912 in 351498d

func performIntToOidCast(

[rafiss]

@fqazi can you provide a description of what failures or issues this caused?

[ajwerner]

This causes the randomized schema change workload to flake because it will no longer see writes. If you continue using a transaction after an error has been swallowed here, the transaction will be internally restarted but the state machine will not observe that. This leads to complete loss of transactionality. This is the greatest source of flakes in #78478 and #78400. This can cause completely incorrect query results.

[ajwerner]

I don’t think this can be a later thing. I can put up a hack, but I don’t feel great about it. I asked Faizan to assign y’all because I think we should have a tight contract about the errors we accept. I don’t know what that set is, and the commentary on the errors we’re willing to accept are lacking. One approach is to just pass through roachpb.TransactionRetryWithProtoRefreshError, which I think is enough, but I’m not certain.

[rafiss]

Thanks for describing the impact. We can pick it up now, but without knowing those details about the impact, we weren't able to make that decision.