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

Don't require a loaded function to have "compile lints" applied. #327

Merged
merged 1 commit into from
Jun 9, 2023

Conversation

eeeebbbbrrrr
Copy link
Contributor

@eeeebbbbrrrr eeeebbbbrrrr commented Jun 9, 2023

Loading an existing, compiled function should not require that it was previously compiled with the set of lints PL/Rust now uses for compilation. This prohibits newer versions of PL/Rust, which might have a new lint, from executing functions created with an older version without that lint.

We do, however, continue to require the lints that are set in both the $PLRUST_REQUIRED_LINTS envar and the plrust.required_lints postgresql.conf GUC. This is a way for the administrator to flat out require certain lints be applied to every function before it can be loaded/executed.

PL/Rust's default stance, however, is that if it was good before, it's good now, even if new lints are applied going forward.

Loading an existing, compiled function should not require that it was
previously compiled with the set of lints PL/Rust now uses for
compilation.  This prohibits newer versions of PL/Rust, which might have
a new lint, from executing functions created with an older version
without that lint.

We do, however, continue to require the lints that are set in both the
`$PLRUST_REQUIRED_LINTS` envar and the `plrust.required_lints`
postgresql.conf GUC.

This is a way for the administrator to flat out **require** that certain
lints be applied to **every** function before it can be loaded/executed.

PL/Rust's default stance, however, is that if it was good before, it's
good now, even if new lints are applied going forward.
Copy link
Contributor

@BradyBonnette BradyBonnette left a comment

Choose a reason for hiding this comment

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

After some offline discussions about this change, I understand them now 👍

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.

2 participants