Macros: add option to surface warning notices as errors

[maximilianthorn]

Bug Description

Given an already existing table, e.g:

CREATE TABLE person (
	first_name TEXT NOT NULL 
);

when adding a second column to the table, and adding a NOT NULL constraint, e.g.

ALTER TABLE person ADD COLUMN last_name TEXT; -- Add column without constraint for existing rows
UPDATE person SET last_name = ''; -- Add default value for existing rows
ALTER TABLE person ALTER COLUMN last_name SET NOT NULL; -- Add constraint

sqlx::query! will not throw a compile time error for the following query:

sqlx::query!(
    r#"INSERT INTO person (first_name)
    VALUES ($1)"#,
    "John"
);

While the new column itself is detected (e.g. for select queries), any later added NOT NULL constraints are apparently completely ignored.

Minimal Reproduction

Minimal github project

Info

• SQLx version: 0.7.4
• SQLx features enabled: ["postgres", "runtime-tokio"]
• Database server and version: psql (PostgreSQL) 15.2 (Debian 15.2-1.pgdg110+1)
• Operating system: Arch Linux 6.6.2-arch1-1 x86_64
• rustc --version: rustc 1.76.0 (07dca489a 2024-02-04)

[abonander]

This is not a bug. If Postgres returns a successful parse result, we have no way of knowing the query is invalid. SQLx doesn't null-check bind parameters or do any sort of analysis on the query itself, only the metadata that Postgres returns.

In this situation, Postgres is likely returning a non-fatal warning message alongside the parse result, which we currently don't have a good way to surface. Unfortunately, emitting lint warnings from proc macros has been unstable for 6 years and doesn't look to be getting stabilized anytime soon. And turning any non-fatal warnings into compiler errors for all users would be an unacceptable breaking change.

We've talked about implementing a Clippy-like linter for SQLx, but that's a whole project in and of itself.

We could use the sqlx_macros_unstable/procmacro2_semver_exempt cfg-flags to conditionally enable the use of the proc_macro::Diagnostic API, but I have a feeling most people don't use those flags,

I think instead, we could make it optional to elevate these warnings to errors, either using an environment variable (e.g. SQLX_DENY_WARNING_NOTICES=1) or by setting a key in a config file committed to the project, e.g. a sqlx.toml.

[maximilianthorn]

Despite having already read issues about SQLx not null-checking, I had the illusion it did in my case and only altering the table was the issue. Yeah, should've definitely rechecked that.

Either way, the latter idea to provide an option to elevate warnings to errors sounds very useful to me. I think the ability to store the option to a file is crucial for reliability (opposed to remembering setting the environment variable every new session).

If using an environment variable includes the option to write it into e.g. the .env file, that sounds like the easier implementation.