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

cast: set volatility of casts to/from TEXT and NAME to leak-proof #81783

Merged
merged 1 commit into from
May 25, 2022

Conversation

mgartner
Copy link
Collaborator

In Postgres 14, some casts which were previously immutable and not
leak-proof are now leak-proof. This commit updates the volatility of
casts to and from TEXT and NAME to leak-proof to match Postgres. This
should aid the optimizer in decorrelating more subqueries. This is not
backward incompatible because it is less restrictive and should not
change the behavior of any users' existing queries.

There are many other casts that can be changed to leak-proof, but our
implementations of those casts need to be audited to ensure that they
cannot err (one of the requirements for leak-proof). I leave this as
future work.

We must also take care with assignment casts, which currently use the
same volatility as a normal cast. Some casts which cannot err and are
leak-proof can err as an assignment cast, so the assignment cast cannot
be leak-proof. For example, a cast like 'foo'::VARCHAR(1) does not
err, but an assignment cast with the same inputs will err because the
value does not fit into the target type's width. To handle cases like
this, we may need to label each cast with two volatilities, one for
regular casts and one for assignment casts.

Release note: None

In Postgres 14, some casts which were previously immutable and not
leak-proof are now leak-proof. This commit updates the volatility of
casts to and from TEXT and NAME to leak-proof to match Postgres. This
should aid the optimizer in decorrelating more subqueries. This is not
backward incompatible because it is less restrictive and should not
change the behavior of any users' existing queries.

There are many other casts that can be changed to leak-proof, but our
implementations of those casts need to be audited to ensure that they
cannot err (one of the requirements for leak-proof). I leave this as
future work.

We must also take care with assignment casts, which currently use the
same volatility as a normal cast. Some casts which cannot err and are
leak-proof can err as an assignment cast, so the assignment cast cannot
be leak-proof. For example, a cast like `'foo'::VARCHAR(1)` does not
err, but an assignment cast with the same inputs will err because the
value does not fit into the target type's width. To handle cases like
this, we may need to label each cast with two volatilities, one for
regular casts and one for assignment casts.

Release note: None
@mgartner mgartner requested review from msirek and a team May 24, 2022 21:16
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@msirek msirek left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!
:lgtm:

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @mgartner)

@mgartner
Copy link
Collaborator Author

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented May 25, 2022

Build succeeded:

@craig craig bot merged commit 85b2ebe into cockroachdb:master May 25, 2022
@mgartner mgartner deleted the leakproof-casts branch May 25, 2022 16:21
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.

3 participants