fix(0011): do not report aggregates as having a mutable search_path - #178
Open
karpovantonme wants to merge 1 commit into
Open
fix(0011): do not report aggregates as having a mutable search_path#178karpovantonme wants to merge 1 commit into
karpovantonme wants to merge 1 commit into
Conversation
`0011_function_search_path_mutable` scans `pg_proc` without filtering
`prokind`, so aggregates are reported alongside functions.
An aggregate's `pg_proc` row is a placeholder: it has no body, `prolang` is
`internal`, and `CREATE AGGREGATE` has no `SET` clause, so `proconfig` is always
null. The lint's "search path not set" condition is therefore always true for
aggregates, and there is no way for a user to resolve the warning.
Reproducing the report:
create function public.uuid_min(uuid, uuid) returns uuid
set search_path = '' language sql immutable strict
as $$ select least($1, $2) $$;
create aggregate public.min_uuid(uuid) (
sfunc = public.uuid_min,
stype = uuid,
combinefunc = public.uuid_min,
parallel = safe
);
`public.min_uuid` is reported even though every function it is built from pins
its search path.
Exclude `prokind = 'a'`. Procedures and window functions are left alone: both
accept a `SET` clause and are still linted. No coverage is lost, because an
aggregate's support functions are ordinary `pg_proc` entries and continue to be
reported on their own -- the test asserts exactly that.
splinter.sql regenerated via bin/compile.py.
Closes supabase#139
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #139.
Problem
0011_function_search_path_mutablescanspg_procwithout filteringprokind, so aggregates are reported alongside functions.An aggregate's
pg_procrow is a placeholder: it has no body,prolangisinternal, andCREATE AGGREGATEhas noSETclause, soproconfigis always null. The lint's "search path not set" condition is therefore always true for an aggregate, and there is no way for a user to resolve the warning.Reproducing the report:
public.min_uuidis reported even though every function it is built from pins its search path.Fix
The issue suggests filtering on
prolang = internalinstead.prokindis the narrower and more direct attribute: it excludes exactly the objects that cannot carry aSETclause, without also excluding internal- or C-language functions, which can.Procedures (
prokind = 'p') and window functions (prokind = 'w') are deliberately left in scope: both accept aSETclause, and a SECURITY DEFINER procedure with a mutable search path is a real finding.No coverage is lost. An aggregate's support functions are ordinary
pg_procentries and are still reported on their own; the test asserts this by dropping theSETclause from the support function and checking that it, and not the aggregate, is flagged.splinter.sqlregenerated viabin/compile.py.Verification
docker compose -f dockerfiles/docker-compose.yml run --rm testonsupabase/postgres:15.1.1.13— all 28 tests pass.pre-commit run --all-files— all hooks pass.