fix: use parameterized queries in RQL utility functions#1547
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR refactors SQL query construction across PostgreSQL repositories by replacing raw SQL string formatting with structured Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Coverage Report for CI Build 24547980609Coverage decreased (-0.001%) to 41.815%Details
Uncovered Changes
Coverage Regressions1 previously-covered line in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/store/postgres/org_projects_repository.go (1)
216-223:⚠️ Potential issue | 🟡 MinorCast target should be
TIMESTAMPTZ, notTIMESTAMP.
projects.created_atis a timezone-aware column (TIMESTAMPTZ) in Postgres, and casting incoming RFC3339 strings toTIMESTAMP(without time zone) discards the offset and interprets the value as a local timestamp. This can yield subtly wrong comparisons across sessions with differingTimeZonesettings.Update the cast and the corresponding expectation in
org_projects_repository_test.go:Proposed fix
- filter.Operator: goqu.Cast(goqu.V(filter.Value), "TIMESTAMP"), + filter.Operator: goqu.Cast(goqu.V(filter.Value), "TIMESTAMPTZ"),Test expectation (line 62):
-`CAST($3 AS TIMESTAMP) +`CAST($3 AS TIMESTAMPTZ)
🧹 Nitpick comments (2)
internal/store/postgres/org_billing_repository_test.go (1)
106-107: Test expectations correctly reflect the newnotemptyexpansion and shifted placeholders.Consider adding a dedicated test case for
like/ilikeoperators on this repo to lock in the new parameterized-pattern semantics (ties into the wildcard-wrapping concern raised onorg_billing_repository.go).internal/store/postgres/org_projects_repository.go (1)
197-199: LGTM on the empty/notempty rewrite.Consistent with the shared helper and the other repositories. One observation (not blocking): this logic is now duplicated across
pkg/utils/rql.go::ProcessStringDataType,org_billing_repository.go::processStringDataType,org_users_repository.go::buildNonRoleFilterCondition, and here. Worth consolidating in a follow-up.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c1b3dc8e-b90e-464b-9ad8-57bdee2c3ca3
📒 Files selected for processing (10)
internal/store/postgres/audit_record_repository.gointernal/store/postgres/org_billing_repository.gointernal/store/postgres/org_billing_repository_test.gointernal/store/postgres/org_projects_repository.gointernal/store/postgres/org_projects_repository_test.gointernal/store/postgres/org_users_repository.gointernal/store/postgres/org_users_repository_test.gointernal/store/postgres/prospect_repository.gointernal/store/postgres/userpat_repository.gopkg/utils/rql.go
Description:
Summary
goqu.L(fmt.Sprintf(...))) with goqu'sparameterized expression API (
goqu.Cast().ILike(),goqu.I().IsNull(), etc.)in shared RQL query builder utilities and repository-level filter functions
Prepared(true)to query builders in audit record, prospect, and user PATrepositories for defense-in-depth
Changes
pkg/utils/rql.go—AddRQLSearchInQueryandProcessStringDataTypenow usegoqu expression builders instead of
goqu.L(fmt.Sprintf(...))internal/store/postgres/org_billing_repository.go— same fix in localprocessStringDataType(covers like, notlike, ilike, notilike, empty, notempty)internal/store/postgres/org_projects_repository.go— same fix inapplyStringFilter(empty/notempty) andapplyDatetimeFilter(timestamp cast)internal/store/postgres/org_users_repository.go— same fix inbuildNonRoleFilterCondition(empty/notempty)Prepared(true)to audit_record, prospect, userpat repositoriesTODO
goqu.SetDefaultPrepared(true)at application startup to enforceprepared statements across all repositories by default
Test plan
make lintpasses with 0 issues