fix: TT-16778 GORM v2 table sharding bug #949

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open

probelabs wants to merge 11 commits into master from TT-16778-fix-gorm-v2-sharding-bug

Open

fix: TT-16778 GORM v2 table sharding bug #949

fix: TT-16778 GORM v2 table sharding bug

probelabs / Visor: quality failed Mar 11, 2026 in 1m 34s

🚨 Check Failed

quality check failed because fail_if condition was met.

Details

📊 Summary

Total Issues: 4
Error Issues: 2
Warning Issues: 2

🔍 Failure Condition Results

Failed Conditions

global_fail_if: output.issues && output.issues.some(i => i.severity === 'critical' || i.severity === 'error')
- Severity: ❌ error

Issues by Category

Logic (3)

❌ 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.
⚠️ 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.
❌ system:0 - Global failure condition met: output.issues && output.issues.some(i => i.severity === 'critical' || i.severity === 'error')

Architecture (1)

⚠️ pumps/sql.go:269 - 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.

Powered by Visor from Probelabs

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

Annotations

Check failure on line 170 in pumps/graph_sql_aggregate.go

probelabs / Visor: quality

logic Issue

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.

Raw output


            Add a fallback for `s.Conf.BatchSize` before the batching loop in `DoAggregatedWriting`.

```go
batchSize := s.Conf.BatchSize
if batchSize == 0 {
    batchSize = SQLDefaultQueryBatchSize // Or another suitable default
}

for i := 0; i < len(recs); i += batchSize {
    // ...
}
```

Check warning on line 283 in pumps/sql.go

probelabs / Visor: quality

architecture Issue

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.

Raw output


            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.

Check warning on line 1 in pumps/sql.go

probelabs / Visor: quality

logic Issue

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.

Raw output


            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.

View more details on probelabs