fix: TT-16778 GORM v2 table sharding bug

[probelabs]

This PR supersedes and closes #948.

Problem / Task

In SQLAggregatePump, SQLPump, and GraphSQLPump, the .WithContext(ctx) call drops the previously applied .Table(table) modifier. This causes the INSERT statement to target the default table instead of the sharded table.

This PR also fixes a CI issue where tests would fail with an empty slice found error if BatchSize was set to 0 in the configuration. A fallback to the default batch size has been added to the batching loops.

Changes

• The .Table(table) modifier is now chained after .WithContext(ctx) to ensure the correct table is targeted.
• Added a fallback for BatchSize in pumps/sql_aggregate.go, pumps/sql.go, and pumps/graph_sql.go to prevent errors with empty slices.

Testing

Added unit tests to verify the fix.

---

Requested by: @U3P2L4XNE | Trace: cc2d9c4a03c8142a19785badffab0fca
Generated with Tyk AI Assistant

Ticket Details

TT-16778

Status	Open
Summary	[Innersource] Aggregated analytics fail to write to sharded tables when MDCB table sharding is enabled with Hybrid Pump

Generated at: 2026-03-11 19:20:04

[probelabs]

This PR fixes a critical bug in SQL-based pumps where enabling table sharding resulted in data being written to the default table instead of the correct date-sharded table. This was caused by an incorrect method order in GORM, where .WithContext(ctx) would reset the previously set table name. The PR corrects the call order to ensure the sharded table is correctly targeted.

Additionally, a separate issue causing CI failures when BatchSize was set to 0 has been resolved by adding a fallback to a default batch size in the data processing loops.

Files Changed Analysis

• Core Logic (pumps/sql.go, pumps/sql_aggregate.go, pumps/graph_sql.go): The primary change is here. The GORM calls are modified to be stateless, applying .Table(table) after .WithContext(ctx) in each Create operation. This prevents the shared database connection from being mutated and ensures the correct table is used. A fallback for a zero BatchSize is also added in these files.
• New Tests (pumps/sql_aggregate_sharding_test.go, pumps/sql_aggregate_sharding_sqlite_test.go): Two new test files have been added to specifically verify the sharding fix. They use go-sqlmock and an in-memory SQLite database to simulate the database interactions and prevent regressions.
• Analytics Helpers (analytics/aggregate.go, analytics/uptime_data.go): The OnConflict assignment functions were refactored to remove a hardcoded table name prefix, making the generated SQL expressions more generic and compatible across different SQL dialects (e.g., SQLite).
• Dependencies (go.mod, go.sum): The go-sqlmock library and its dependencies have been added to support the new tests.

Architecture & Impact Assessment

• What this PR accomplishes: It resolves a significant data integrity bug for users relying on SQL table sharding with SQLPump, SQLAggregatePump, or GraphSQLPump. It ensures analytics are stored in the correct date-partitioned tables and improves the robustness of the data processing loop against invalid configurations.
• Key technical changes introduced:
  1. Corrected GORM Method Chain: The order of GORM calls is now db.WithContext(ctx).Table(table)..., which correctly applies the context and then sets the target table for the operation.
  2. Stateless DB Operations: The code no longer mutates the shared pump database connection (c.db = c.db.Table(...)). Instead, the target table is specified per-operation, reducing side effects.
  3. Configuration Fallback: A check has been added to use a default batch size if the configured BatchSize is zero, preventing errors with empty slice operations.
• Affected system components: The data persistence layer for SQLPump, SQLAggregatePump, and GraphSQLPump is affected. This is a critical fix for any user employing a SQL backend (e.g., PostgreSQL, CockroachDB) with TableSharding enabled.

sequenceDiagram
    participant Pump
    participant GORM
    participant Database

    Note over Pump, GORM: Before Fix (Incorrect Order)
    Pump->>GORM: .Table("sharded_table_20260311")
    GORM->>GORM: Sets target table
    Pump->>GORM: .WithContext(ctx)
    GORM->>GORM: Resets target table to default!
    Pump->>GORM: .Create(...)
    GORM->>Database: INSERT INTO "default_analytics_table"

    Note over Pump, GORM: After Fix (Correct Order)
    Pump->>GORM: .WithContext(ctx)
    GORM->>GORM: Sets context
    Pump->>GORM: .Table("sharded_table_20260311")
    GORM->>GORM: Sets target table
    Pump->>GORM: .Create(...)
    GORM->>Database: INSERT INTO "sharded_table_20260311"

Loading

Scope Discovery & Context Expansion

• The impact of this bug is confined to the pumps package, specifically affecting the SQL-based analytics pumps. It addresses a data-layer bug that compromises the integrity of stored analytics when table sharding is active.
• The fix is critical for users who partition analytics data by date in SQL databases, a common strategy for managing large data volumes.
• For full context, one would examine config.go, where the TableSharding and BatchSize configurations are defined, and main.go, which handles the initialization and lifecycle of these pumps.

Metadata

• Review Effort: 2 / 5
• Primary Label: bug

---

Powered by Visor from Probelabs

Last updated: 2026-03-11T19:21:55.674Z | Triggered by: pr_updated | Commit: cfadd4b

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

[probelabs]

✅ Security Check Passed

No security issues found – changes LGTM.

Architecture Issues (2)

Severity	Location	Issue
🟡 Warning	pumps/sql.go:272-278	The batch processing loop logic is duplicated across multiple files (`sql.go`, `sql_aggregate.go`, `graph_sql.go`). This includes checking for a zero batch size, setting a default, and iterating through slices. This duplicated code increases maintenance overhead, as any future changes to the batching logic will need to be applied in multiple places. 💡 Suggestion To improve reusability and adhere to the DRY (Don't Repeat Yourself) principle, extract the batching logic into a shared helper function. This function could accept the total number of records, the configured batch size, and a callback function to execute for each batch. This centralizes the logic, making it easier to maintain and test.
🟡 Warning	pumps/sql.go:374	The method for checking the SQL dialect is inconsistent across different pumps. Here, `c.dbType != "sqlite"` is used, which relies on a string field in the pump's configuration. However, in `pumps/graph_sql_aggregate.go:186`, the check is `s.db.Dialector.Name() != "sqlite"`, which queries the GORM dialector directly. Using the GORM dialector is a more robust and reliable approach as it reflects the actual database connection's state, whereas a stored string could potentially become stale or inconsistent. 💡 Suggestion For better consistency and reliability, standardize on a single method for checking the database dialect. It is recommended to consistently use `db.Dialector.Name()` across all SQL pumps, as this directly queries the active GORM connection for its type.

✅ Performance Check Passed

No performance issues found – changes LGTM.

✅ Security Check Passed

No security issues found – changes LGTM.

\n\n

Architecture Issues (2)

Severity	Location	Issue
🟡 Warning	pumps/sql.go:272-278	The batch processing loop logic is duplicated across multiple files (`sql.go`, `sql_aggregate.go`, `graph_sql.go`). This includes checking for a zero batch size, setting a default, and iterating through slices. This duplicated code increases maintenance overhead, as any future changes to the batching logic will need to be applied in multiple places. 💡 Suggestion To improve reusability and adhere to the DRY (Don't Repeat Yourself) principle, extract the batching logic into a shared helper function. This function could accept the total number of records, the configured batch size, and a callback function to execute for each batch. This centralizes the logic, making it easier to maintain and test.
🟡 Warning	pumps/sql.go:374	The method for checking the SQL dialect is inconsistent across different pumps. Here, `c.dbType != "sqlite"` is used, which relies on a string field in the pump's configuration. However, in `pumps/graph_sql_aggregate.go:186`, the check is `s.db.Dialector.Name() != "sqlite"`, which queries the GORM dialector directly. Using the GORM dialector is a more robust and reliable approach as it reflects the actual database connection's state, whereas a stored string could potentially become stale or inconsistent. 💡 Suggestion For better consistency and reliability, standardize on a single method for checking the database dialect. It is recommended to consistently use `db.Dialector.Name()` across all SQL pumps, as this directly queries the active GORM connection for its type.

\n\n \n\n

Quality Issues (3)

Severity	Location	Issue
🟠 Error	pumps/graph_sql_aggregate.go:170	The batching loop in `DoAggregatedWriting` does not handle a `BatchSize` of zero, which will cause an infinite loop and prevent the pump from processing data. A fallback to a default batch size should be added, similar to the fixes applied in `pumps/sql_aggregate.go` and other pumps in this pull request. 💡 Suggestion Add a fallback for `s.Conf.BatchSize` before the batching loop in `DoAggregatedWriting`. batchSize := s.Conf.BatchSize if batchSize == 0 { batchSize = SQLDefaultQueryBatchSize // Or another suitable default } for i := 0; i < len(recs); i += batchSize { // ... }
🟡 Warning	pumps/sql.go:269-283	The batch processing logic, including checking for a zero batch size, calculating batch boundaries, and iterating over a slice, is duplicated across multiple pump implementations (`sql.go`, `sql_aggregate.go`, `graph_sql.go`, `graph_sql_aggregate.go`). This duplication increases maintenance effort and has already led to inconsistent implementation, as the zero-batch-size fix was missed in `pumps/graph_sql_aggregate.go`. This logic should be centralized into a single, reusable helper function. 💡 Suggestion Refactor the duplicated batching logic into a generic helper function. This would centralize the logic, reduce code duplication, and ensure that fixes and improvements are applied consistently across all pumps.
🟡 Warning	pumps/sql.go:1	The pull request applies critical fixes for a GORM sharding bug and a zero-batch-size issue to multiple pumps, including `SQLPump` (`pumps/sql.go`), `GraphSQLPump` (`pumps/graph_sql.go`), and `GraphSQLAggregatePump` (`pumps/graph_sql_aggregate.go`). However, the new tests only cover the changes in `SQLAggregatePump`. The absence of tests for the other modified pumps means these fixes are not verified and are susceptible to future regressions. 💡 Suggestion Add unit or integration tests for `SQLPump`, `GraphSQLPump`, and `GraphSQLAggregatePump` to verify that the table sharding and batch size fixes work correctly. The tests added for `SQLAggregatePump` can serve as a template.

---

Powered by Visor from Probelabs

Last updated: 2026-03-11T19:21:45.417Z | Triggered by: pr_updated | Commit: cfadd4b

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

[github-actions]

🚨 Jira Linter Failed

Commit: cfadd4b
Failed at: 2026-03-11 19:20:07 UTC

The Jira linter failed to validate your PR. Please check the error details below:

🔍 Click to view error details

failed to validate Jira issue: jira ticket TT-16778 has status 'Open' but must be one of: In Dev, In Code Review, Ready For Dev, Dod Check

Next Steps

• Ensure your branch name contains a valid Jira ticket ID (e.g., ABC-123)
• Verify your PR title matches the branch's Jira ticket ID
• Check that the Jira ticket exists and is accessible

---

This comment will be automatically deleted once the linter passes.

[sonarqubecloud]

Quality Gate failed

Failed conditions
26.6% Coverage on New Code (required ≥ 80%)
8.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud