From ed93301a1fdda2915fd0ed0bfa2bb792a8edb397 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89mile=20R=C3=A9?= Date: Thu, 28 May 2026 21:36:04 +0200 Subject: [PATCH] Use subquery for common third party filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the two-step ID-materializing pattern (fetch IDs in Go, pass as ANY(@ids)) with an IN-subquery that keeps the filtering entirely in the database and eliminates an extra round trip. Remove the now unused LoadIDsByCommonThirdPartyID and its service wrapper. Update the coredata rule to clarify that subqueries for filtering are OK. Signed-off-by: Émile Ré --- .cursor/rules/go-coredata-load-naming.mdc | 25 +++++-- pkg/cookiebanner/service.go | 34 --------- pkg/coredata/common_tracker_pattern.go | 33 --------- pkg/coredata/tracker_pattern_filter.go | 71 +++++++++---------- .../api/console/v1/cookie_banner_resolvers.go | 12 +--- 5 files changed, 55 insertions(+), 120 deletions(-) diff --git a/.cursor/rules/go-coredata-load-naming.mdc b/.cursor/rules/go-coredata-load-naming.mdc index a20241da0..456d85c0e 100644 --- a/.cursor/rules/go-coredata-load-naming.mdc +++ b/.cursor/rules/go-coredata-load-naming.mdc @@ -58,15 +58,20 @@ func (ds *Things) LoadByParentID( ) error { ``` -# No cross-entity JOINs +# Cross-entity table references -Each entity file in `pkg/coredata` queries only its own table. When data from multiple entities is needed, the caller orchestrates separate calls. +Each entity file in `pkg/coredata` queries its own table. Never JOIN two +entity tables to **return columns from both** — the caller orchestrates +separate calls instead. -- Never JOIN two entity tables inside an entity method. -- Never return a raw ID belonging to a different entity — return the full entity and let the caller read the foreign key field. +**Subqueries for filtering are OK.** When the only purpose of the other +table is to narrow a `WHERE` clause (e.g. `IN (SELECT id FROM ...)`), +keep it in the query rather than materializing IDs in Go and passing +them as an `ANY(@ids)` parameter. A subquery keeps the filtering in the +database and eliminates an extra round trip. ```go -// BAD — cross-entity JOIN inside DetectedTrackers +// BAD — cross-entity JOIN that returns columns from both tables q := ` SELECT ctpd.common_third_party_id FROM detected_trackers dt @@ -80,4 +85,14 @@ filter := coredata.NewCommonThirdPartyDomainFilter(domains) var matched coredata.CommonThirdPartyDomains err = matched.Load(ctx, conn, 1, filter) thirdPartyID := matched[0].CommonThirdPartyID + +// GOOD — subquery only used for filtering, no columns returned from it +q := ` +SELECT id, pattern, tracker_type, ... +FROM tracker_patterns +WHERE common_tracker_pattern_id IN ( + SELECT id FROM common_tracker_patterns + WHERE common_third_party_id = @filter_common_third_party_id +) +` ``` diff --git a/pkg/cookiebanner/service.go b/pkg/cookiebanner/service.go index 20a79428a..12067126c 100644 --- a/pkg/cookiebanner/service.go +++ b/pkg/cookiebanner/service.go @@ -2746,40 +2746,6 @@ func (s *Service) GetCommonTrackerPatternsByIDs( return patterns, nil } -// LoadCommonTrackerPatternIDsByCommonThirdPartyID returns the IDs of -// every common tracker pattern referencing the given common third party. -// Used by the trackers list filter to translate a CommonThirdParty GID -// into a `common_tracker_pattern_id = ANY(...)` constraint without -// JOINing across entity tables in coredata. -func (s *Service) LoadCommonTrackerPatternIDsByCommonThirdPartyID( - ctx context.Context, - commonThirdPartyID gid.GID, -) ([]gid.GID, error) { - var ids []gid.GID - - err := s.pg.WithConn( - ctx, - func(ctx context.Context, conn pg.Querier) error { - var ( - patterns coredata.CommonTrackerPatterns - err error - ) - - ids, err = patterns.LoadIDsByCommonThirdPartyID(ctx, conn, commonThirdPartyID) - if err != nil { - return fmt.Errorf("cannot load common tracker pattern ids: %w", err) - } - - return nil - }, - ) - if err != nil { - return nil, err - } - - return ids, nil -} - // LoadDistinctThirdPartyIDsByCookieBannerID returns the distinct // org-scoped third-party IDs referenced by tracker patterns of the // banner. The companion diff --git a/pkg/coredata/common_tracker_pattern.go b/pkg/coredata/common_tracker_pattern.go index 505124285..5b88baa60 100644 --- a/pkg/coredata/common_tracker_pattern.go +++ b/pkg/coredata/common_tracker_pattern.go @@ -483,36 +483,3 @@ WHERE return nil } - -// LoadIDsByCommonThirdPartyID returns just the IDs of the common tracker -// patterns linked to the given common third party. Callers use it to feed -// a `common_tracker_pattern_id = ANY(...)` filter on tracker_patterns -// without crossing the entity boundary. -func (ps *CommonTrackerPatterns) LoadIDsByCommonThirdPartyID( - ctx context.Context, - conn pg.Querier, - commonThirdPartyID gid.GID, -) ([]gid.GID, error) { - q := ` -SELECT - id -FROM - common_tracker_patterns -WHERE - common_third_party_id = @common_third_party_id -` - - args := pgx.StrictNamedArgs{"common_third_party_id": commonThirdPartyID} - - rows, err := conn.Query(ctx, q, args) - if err != nil { - return nil, fmt.Errorf("cannot query common tracker pattern ids: %w", err) - } - - ids, err := pgx.CollectRows(rows, pgx.RowTo[gid.GID]) - if err != nil { - return nil, fmt.Errorf("cannot collect common tracker pattern ids: %w", err) - } - - return ids, nil -} diff --git a/pkg/coredata/tracker_pattern_filter.go b/pkg/coredata/tracker_pattern_filter.go index 77f6ee1a4..7fc6052f1 100644 --- a/pkg/coredata/tracker_pattern_filter.go +++ b/pkg/coredata/tracker_pattern_filter.go @@ -20,14 +20,14 @@ import ( ) type TrackerPatternFilter struct { - matchType *TrackerPatternMatchType - cookieCategoryID *gid.GID - excluded *bool - query *string - source *CookieSource - trackerType *TrackerType - thirdPartyID *gid.GID - commonTrackerPatternIDs []gid.GID + matchType *TrackerPatternMatchType + cookieCategoryID *gid.GID + excluded *bool + query *string + source *CookieSource + trackerType *TrackerType + thirdPartyID *gid.GID + commonThirdPartyID *gid.GID } func NewTrackerPatternFilter( @@ -62,14 +62,8 @@ func (f *TrackerPatternFilter) WithThirdPartyID(thirdPartyID *gid.GID) *TrackerP return f } -// WithCommonTrackerPatternIDs constrains the result to tracker patterns -// whose `common_tracker_pattern_id` is in the given set. Callers -// pre-resolve this list (typically via -// CommonTrackerPatterns.LoadIDsByCommonThirdPartyID) so the filter stays -// inside the tracker_patterns table. Passing an empty (non-nil) slice -// yields no rows. -func (f *TrackerPatternFilter) WithCommonTrackerPatternIDs(ids []gid.GID) *TrackerPatternFilter { - f.commonTrackerPatternIDs = ids +func (f *TrackerPatternFilter) WithCommonThirdPartyID(id *gid.GID) *TrackerPatternFilter { + f.commonThirdPartyID = id return f } @@ -130,9 +124,12 @@ func (f *TrackerPatternFilter) SQLFragment() string { END AND CASE - WHEN @has_common_tracker_pattern_ids_filter::boolean = false THEN TRUE - WHEN @has_common_tracker_pattern_ids_filter::boolean = true THEN - common_tracker_pattern_id = ANY(@filter_common_tracker_pattern_ids::text[]) + WHEN @has_common_third_party_id_filter::boolean = false THEN TRUE + WHEN @has_common_third_party_id_filter::boolean = true THEN + common_tracker_pattern_id IN ( + SELECT id FROM common_tracker_patterns + WHERE common_third_party_id = @filter_common_third_party_id::text + ) ELSE TRUE END )` @@ -144,21 +141,21 @@ func (f *TrackerPatternFilter) SQLArguments() pgx.StrictNamedArgs { } args := pgx.StrictNamedArgs{ - "has_match_type_filter": false, - "filter_match_type": nil, - "has_cookie_category_id_filter": false, - "filter_cookie_category_id": nil, - "has_excluded_filter": false, - "filter_excluded": nil, - "filter_query": nil, - "has_source_filter": false, - "filter_source": nil, - "has_tracker_type_filter": false, - "filter_tracker_type": nil, - "has_third_party_id_filter": false, - "filter_third_party_id": nil, - "has_common_tracker_pattern_ids_filter": false, - "filter_common_tracker_pattern_ids": []gid.GID{}, + "has_match_type_filter": false, + "filter_match_type": nil, + "has_cookie_category_id_filter": false, + "filter_cookie_category_id": nil, + "has_excluded_filter": false, + "filter_excluded": nil, + "filter_query": nil, + "has_source_filter": false, + "filter_source": nil, + "has_tracker_type_filter": false, + "filter_tracker_type": nil, + "has_third_party_id_filter": false, + "filter_third_party_id": nil, + "has_common_third_party_id_filter": false, + "filter_common_third_party_id": nil, } if f.matchType != nil { @@ -195,9 +192,9 @@ func (f *TrackerPatternFilter) SQLArguments() pgx.StrictNamedArgs { args["filter_third_party_id"] = *f.thirdPartyID } - if f.commonTrackerPatternIDs != nil { - args["has_common_tracker_pattern_ids_filter"] = true - args["filter_common_tracker_pattern_ids"] = f.commonTrackerPatternIDs + if f.commonThirdPartyID != nil { + args["has_common_third_party_id_filter"] = true + args["filter_common_third_party_id"] = *f.commonThirdPartyID } return args diff --git a/pkg/server/api/console/v1/cookie_banner_resolvers.go b/pkg/server/api/console/v1/cookie_banner_resolvers.go index bf0d7f832..b7fdd2fba 100644 --- a/pkg/server/api/console/v1/cookie_banner_resolvers.go +++ b/pkg/server/api/console/v1/cookie_banner_resolvers.go @@ -217,17 +217,7 @@ func (r *cookieBannerResolver) TrackerPatterns(ctx context.Context, obj *types.C case coredata.ThirdPartyEntityType: coredataFilter = coredataFilter.WithThirdPartyID(filter.ThirdPartyID) case coredata.CommonThirdPartyEntityType: - ids, err := r.cookieBanner.LoadCommonTrackerPatternIDsByCommonThirdPartyID(ctx, *filter.ThirdPartyID) - if err != nil { - r.logger.ErrorCtx(ctx, "cannot resolve common third party tracker patterns", log.Error(err)) - return nil, gqlutils.Internal(ctx) - } - - if ids == nil { - ids = []gid.GID{} - } - - coredataFilter = coredataFilter.WithCommonTrackerPatternIDs(ids) + coredataFilter = coredataFilter.WithCommonThirdPartyID(filter.ThirdPartyID) default: return nil, gqlutils.Invalidf(ctx, "thirdPartyId must reference a ThirdParty or CommonThirdParty") }