TT-17841: POC — Postgres analytics-aggregation support ($cond, operators)

[sredxny]

Description (POC — draft, stacked on #158)

Part of the TT-17841 POC to let Tyk consumers (starting with tyk-analytics) run
their analytics aggregations on Postgres through the same
Aggregate(ctx, row, pipeline []DBM) interface that already works on MongoDB —
the Mongo driver passes the pipeline to the server's native engine, while the
Postgres driver must translate it to SQL, and that translator was too limited.

Base branch is improve-tests-fix-postgres-transactions (#158) so this diff
shows only the POC changes. Rebase onto main after #158 merges.

B1 — raw-log & uptime reports (implemented)

• $sum: {$cond: …} conditional accumulators → SUM(CASE WHEN … END), the
  idiom analytics use for success/error/response-code rollups. Includes a
  boolean-expression compiler ($and/$or/$eq/$ne/$gt/$gte/$lt/$lte),
  nested $cond, field refs and literals; object and array $cond forms.
• Hardening: aggregation field references are validated
  (sanitizeAggField), closing the old fmt.Sprintf injection vector; numeric
  literals are inlined so SELECT-list expressions don't disturb WHERE parameter
  ordering.
• translateQuery operator fixes (CRUD $match, surfaced by the tyk-sink
  POC): $ne now includes NULL rows to match Mongo semantics
  ((col IS NULL OR col <> ?)); $regex/$options → ~/~*; $exists →
  IS [NOT] NULL.

B2 — pre-aggregated / graph report (attempted)

Determined it requires schema alignment: Mongo stores counters in arrays
($lists.*, $apikeys.<key>) and unwinds them, while the SQL schema stores one
row per element keyed by dimension/dimension_value. $unwind therefore has
no single-table SQL rewrite, so it is now rejected with an actionable error
rather than silently producing a wrong query. Design + remaining work in
docs/postgres-analytics-aggregation.md.

Tests

• TestTranslateAggregationConditional — string-level (no DB): conditional
  counts, $cond array form, identifier rejection, unsupported-operator and
  $unwind errors.
• Existing DB-backed TestTranslateQuery / TestAggregate / conformance suites
  pass against Postgres 16 (validated locally); golangci-lint clean.

Types of changes

• New feature (non-breaking) — additive translator support
• Bug fix — $ne NULL semantics

🤖 Generated with Claude Code

[github-actions]

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.

---

I have read the CLA Document and I hereby sign the CLA

---

1 out of 2 committers have signed the CLA.
✅ @sredxny
❌ @sredny Buitrago
Sredny Buitrago seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You can retrigger this bot by commenting recheck in this Pull Request}

[probelabs]

This Proof of Concept (POC) significantly enhances the Postgres driver by introducing support for complex MongoDB-style analytics aggregation pipelines. The primary goal is to allow services like tyk-analytics to run their aggregations on Postgres using the same Aggregate interface that already works on MongoDB.

The core of this PR is a new translator that converts conditional aggregation operators ($cond) and their nested boolean expressions into SQL CASE statements. This is crucial for analytics reports that compute success/error rollups based on conditions.

Key enhancements include:

• Conditional Aggregation: Translates $sum: {$cond: ...} expressions into SQL SUM(CASE WHEN ... END).
• Security Hardening: Introduces sanitizeAggField to validate field identifiers in aggregation expressions, preventing SQL injection.
• Operator Compatibility: Aligns the behavior of $ne, $regex, and $exists operators with MongoDB semantics (e.g., $ne now correctly handles NULL values).
• Unsupported Operator Handling: The $unwind operator is now explicitly rejected with a clear, actionable error message, preventing silent failures due to schema differences between the document and relational models.
• Date Sharding: The aggregation pipeline now supports date-based table sharding via a _date_sharding directive, fanning out queries across multiple daily tables using UNION ALL.
• Connection Management: The PersistentStorage interface now includes a Close() method, and the lifecycle management for Postgres connections has been improved.

Files Changed Analysis

The changes are primarily focused on the Postgres driver located in persistent/internal/driver/postgres/.

• persistent/internal/driver/postgres/query.go: Contains the bulk of the new logic, with over 600 lines added to implement the aggregation pipeline translator.
• persistent/internal/driver/postgres/query_test.go: A comprehensive test suite with over 300 new lines validates the new translation capabilities, including conditional aggregations, operator fixes, and error handling for unsupported operators.
• docs/postgres-analytics-aggregation.md: A new document provides essential context on the feature, its current limitations (like $unwind), and the path forward.
• Supporting changes in indexes.go, lifecycle.go, schema.go, and the types directory improve index namespacing, connection management, make schema migrations more robust, and add the necessary configuration options.

Architecture & Impact Assessment

• What this PR accomplishes: It bridges a significant feature gap between the MongoDB and Postgres drivers, making Postgres a more viable backend for Tyk components that rely on complex data aggregation for analytics.

• Key technical changes introduced:

  1. Aggregation Expression Translator: A new component recursively translates MongoDB's $cond expressions and nested boolean/comparison operators ($or, $eq, $gte, etc.) into SQL CASE WHEN ... END constructs.
  2. Date-Sharding Resolution: The new resolveAggregateFrom function inspects the pipeline for a _date_sharding directive and dynamically builds a UNION ALL from-clause to query across the relevant sharded tables.
  3. Security Hardening: The sanitizeAggField function validates and sanitizes all field names used within aggregation expressions to mitigate SQL injection risks.
  4. Explicit Rejection of $unwind: The translator proactively identifies and rejects the $unwind operator. This is a deliberate design choice that prevents incorrect query generation and provides developers with a clear error explaining the underlying schema divergence.

• Affected system components: The primary impact is on the persistent storage layer, specifically the Postgres driver. This directly benefits any consumer of this driver that performs analytics, most notably tyk-analytics and tyk-sink.

Aggregation Translation Flow

graph TD
    subgraph "Application (e.g., Tyk Analytics)"
        A["Aggregate(pipeline)"]
    end

    subgraph "Storage Driver (Postgres)"
        B["resolveAggregateFrom()"]
        C["translateAggregationPipeline()"]
        D{"Recursive Expression Translator"}
    end

    subgraph "Database"
        E["Postgres SQL Query"]
        F["FROM (UNION ALL) / GROUP BY / WHERE"]
    end

    A --|MongoDB-style pipeline|--> B
    B --|Resolves sharded tables|--> C
    C --|Parses stages and expressions|--> D
    D --|e.g. $cond becomes CASE WHEN...|--> C
    C --|Generates SQL|--> E
    E --> F

Loading

Scope Discovery & Context Expansion

This PR is a foundational step in a broader strategy to achieve feature parity between the MongoDB and Postgres backends. Its impact extends beyond this single feature:

• The fixes to translateQuery (especially for $ne to include NULLs) improve the correctness of general CRUD operations for any service using the Postgres driver, not just analytics.
• The explicit rejection of $unwind highlights a fundamental architectural decision point. It surfaces a schema divergence that must be addressed for reports that rely on un-nesting data (e.g., pre-aggregated/graph reports). The new documentation correctly frames this as a separate, larger effort, providing clarity for future planning.
• To fully understand the impact, the next logical step would be to examine the aggregation pipelines within the tyk-analytics codebase. This would reveal which specific analytics reports are now fully supported on Postgres and which remain blocked by unsupported operators like $unwind or $dateTrunc, creating a clear roadmap for subsequent work.

Metadata

• Review Effort: 4 / 5
• Primary Label: feature

---

Powered by Visor from Probelabs

Last updated: 2026-08-13T20:20:24.195Z | Triggered by: pr_updated | Commit: eebccb3

💡 TIP: You can chat with Visor using /visor ask <your question>

[probelabs]

Security Issues (1)

Severity	Location	Issue
🔴 Critical	persistent/internal/driver/postgres/query.go:322-1391	The functions `translateQuery` and `buildWhereClause` are vulnerable to SQL injection. Field names, which can be controlled by user input through the `model.DBM` query object, are directly interpolated into SQL queries using `fmt.Sprintf` without sanitization. While the pull request introduces a `sanitizeAggField` function for hardening, it is not consistently applied, particularly for field names in simple queries (`translateQuery`) and in the `$match` stage of aggregation pipelines (`buildWhereClause`). An attacker could provide a malicious field name like `"name"; DROP TABLE users; --"` to execute arbitrary SQL commands. 💡 Suggestion Sanitize all field name keys read from the input `model.DBM` in both `translateQuery` and `buildWhereClause` using the newly introduced `sanitizeAggField` function. This should be done before the keys are used in any `fmt.Sprintf` call that constructs a SQL fragment. The signatures of these functions may need to be updated to propagate errors from the sanitization function. For translateQuery, sanitize the key k at the top of the for k, v := range q loop. For buildWhereClause, change its signature to func buildWhereClause(filter model.DBM) (string, []interface{}, error), sanitize the key k within its loop, and handle the returned error in its caller, translateAggregationPipeline.

Architecture Issues (3)

Severity	Location	Issue
🔴 Critical	persistent/internal/driver/postgres/query.go:505	The `shardedFrom` function constructs a `UNION ALL` query by concatenating table names read directly from `pg_tables`. These table names are not quoted. If an attacker has privileges to create tables, they could create a table with a specially crafted name (e.g., `"my_table; DROP users;"`) that would lead to SQL injection when interpolated into the query string. All identifiers from external sources (even the database schema) should be properly quoted before being used in dynamic SQL. 💡 Suggestion Properly quote the `tableName` variable before concatenating it into the `allTablesSQL` slice. The GORM dialector should provide a quoting function, or you can use a library-provided function like `pq.QuoteIdentifier` if available, to ensure table names are safely included in the query.
🟠 Error	persistent/internal/driver/postgres/query.go:558-565	The aggregation translator does not quote identifiers used in the `GROUP BY` clause. This is inconsistent with the handling of `SELECT` aliases and `ORDER BY` columns, which are correctly quoted. This omission can cause SQL syntax errors if a field name used for grouping is a reserved keyword (e.g., "user", "order"). 💡 Suggestion Ensure all column identifiers in the `GROUP BY` clause are quoted. Modify the logic to quote each field name before it is added to the `groupByClause` string, similar to how it's done for the `ORDER BY` clause.
🟡 Warning	persistent/internal/driver/postgres/query.go:508	The `translateAggregationPipeline` function is excessively long and handles the logic for all aggregation stages within a single large `switch` statement. This high complexity makes the function difficult to read, maintain, and test effectively. 💡 Suggestion Refactor the logic for each pipeline stage (`$match`, `$group`, `$project`, etc.) into its own dedicated helper function. This will break down the complexity of `translateAggregationPipeline`, improve its readability, and allow for more granular and focused unit testing of each stage's translation logic.

Performance Issues (2)

Severity	Location	Issue
🟡 Warning	persistent/internal/driver/postgres/query.go:126-567	The `Aggregate` function calls `resolveAggregateFrom` for every request. When date sharding is used, this in turn calls `shardedFrom`, which executes a `SELECT` query against the `pg_tables` system catalog to discover shard tables. This query is executed for every sharded aggregation, which can cause unnecessary database load and add latency, especially if aggregations are frequent or the database contains many tables. 💡 Suggestion The list of tables in a database schema changes infrequently. Consider caching the results of the `pg_tables` query for a short duration (e.g., 1-5 minutes) within the driver to avoid repeated lookups for the same base table pattern. This would significantly reduce the overhead for frequent sharded aggregation queries.
🟡 Warning	persistent/internal/driver/postgres/query.go:173-789	The `normalizeAggregateValue` function is called for every cell in the aggregation result set. It performs type-switching and, for numeric types returned as strings or byte slices by the driver (e.g., `NUMERIC`), it performs string-to-number parsing using `strconv`. This client-side processing can consume significant CPU and add latency when processing large result sets, as string parsing is computationally more expensive than handling native numeric types. 💡 Suggestion Investigate if the database driver and GORM configuration can be tuned to return native Go numeric types (`int64`, `float64`) directly for standard aggregate functions like `SUM`, `COUNT`, and `AVG` on numeric columns, which would avoid the parsing overhead. If this is not possible due to the need to support arbitrary-precision `NUMERIC` types, this overhead is a known trade-off, but it should be documented. Another option could be to check the column type from `rows.ColumnTypes()` and perform the conversion only for specific types known to cause this issue.

Quality Issues (5)

Severity	Location	Issue
🔴 Critical	persistent/internal/driver/postgres/query.go:326-366	A potential SQL injection vulnerability exists in the `translateQuery` function. Field names (`k`) from the user-provided query map (`q`) are directly interpolated into SQL query strings using `fmt.Sprintf` without proper sanitization. GORM only escapes query parameters (`?`), not the format string itself, leaving the application vulnerable to injection through crafted field names in the query. 💡 Suggestion Sanitize the field name `k` at the beginning of the loop before it is used in any `fmt.Sprintf` call that constructs SQL. A validation function, similar to the newly added `sanitizeAggField`, should be used to ensure that `k` is a valid and safe column identifier. All occurrences where `k` is formatted into a SQL string (e.g., for operators `$ne`, `$gt`, `$lt`, etc.) are affected.
🔴 Critical	persistent/internal/driver/postgres/query.go:775-863	A potential SQL injection vulnerability exists in the `buildWhereClause` function, which is used by the `$match` stage of an aggregation pipeline. Field names (`k`) from the filter map are used in `fmt.Sprintf` to construct SQL conditions without being validated as safe identifiers. While dots are replaced with underscores, no further validation is performed, allowing malicious input to be injected into the generated WHERE clause. 💡 Suggestion Apply a sanitization function to the key `k` after replacing dots with underscores. The new `sanitizeAggField` function can be adapted for this purpose. The function should validate that the resulting key is a simple, safe identifier before it is interpolated into the SQL string.
🟡 Warning	persistent/internal/driver/postgres/query.go:1	The `query.go` file has grown to over 800 lines and now contains two large, distinct responsibilities: translating standard CRUD queries and translating complex aggregation pipelines. This reduces readability and maintainability. 💡 Suggestion Refactor the aggregation pipeline translation logic into a separate file within the same package (e.g., `aggregate.go`). This would include `translateAggregationPipeline` and its many helper functions (`translateAggValueExpr`, `translateCondExpr`, `translateAggBoolExpr`, `sanitizeAggField`, etc.), improving separation of concerns and making the codebase easier to navigate.
🟡 Warning	persistent/internal/driver/postgres/lifecycle.go:102	The error message "closing a no connected database" is grammatically unconventional. 💡 Suggestion Improve the clarity and grammar of the error message. Consider changing it to "database is not connected" or "cannot close a non-connected database".
🟡 Warning	persistent/internal/driver/postgres/query_test.go:1098-1109	The helper function `toInt64` handles multiple numeric types but uses `fmt.Sscan` for `[]byte` and `string` conversions. This can silently ignore trailing non-numeric characters (e.g., "120abc" would parse as 120). While unlikely with database results, using `strconv.ParseInt` would provide stricter parsing and better error handling. 💡 Suggestion Replace `fmt.Sscan` with `strconv.ParseInt` for `[]byte` and `string` types to ensure the entire value is a valid integer. This makes the test helper more robust.

---

Powered by Visor from Probelabs

Last updated: 2026-08-13T20:19:47.432Z | Triggered by: pr_updated | Commit: eebccb3

💡 TIP: You can chat with Visor using /visor ask <your question>

[sonarqubecloud]

Quality Gate failed

Failed conditions
77.9% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud