Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 34 additions & 3 deletions docs/databases.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ another dialect means adding a profile to `scripts/dialect-convert/profiles.py`,
not writing a second converter — `profiles.py` already carries a starting sketch
for SQL Server.

Current state: **1471 of 1471 translated statements apply cleanly, producing 273
Current state: **1518 of 1518 translated statements apply cleanly, producing 273
tables.**

### What the converter translates
Expand All @@ -55,7 +55,7 @@ tables.**
| `jsonb` / `json` | `TEXT` |
| `varchar(n)`, `uuid`, `timestamptz`, ranges | `TEXT` |
| `bigint`, `smallint`, `boolean`, identity columns | `INTEGER` |
| `numeric(p,s)` | `NUMERIC` |
| `numeric(p,s)` | `REAL` (NUMERIC affinity demotes integers and breaks float64 scans) |
| `bytea` | `BLOB` |
| `EXTRACT(EPOCH FROM CURRENT_TIMESTAMP)::bigint` | `(unixepoch())` |
| `now()` | `CURRENT_TIMESTAMP` |
Expand Down Expand Up @@ -83,7 +83,7 @@ Every dropped statement is reported by reason rather than silently discarded.
| `EXCLUDE USING gist` constraints | no exclusion constraints |
| `ALTER TABLE ... ADD CONSTRAINT` | SQLite cannot add constraints after creation |
| `ALTER TABLE ... ALTER COLUMN` | SQLite cannot alter column definitions |
| `ALTER TABLE ... DROP COLUMN` | rejected whenever an index or `CHECK` still references the column |
| `ALTER TABLE ... DROP COLUMN` | only when an index or constraint still pins the column; otherwise it is emitted, dropping blocking indexes first |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Correct the DROP COLUMN condition.

Line 86 says the statement is emitted only when an index or constraint still references the column, then says it is emitted otherwise. This reverses the documented behavior. State that the converter emits the statement after removing blocking indexes when no index or constraint still references the column; otherwise it drops the statement.

Proposed wording
-| `ALTER TABLE ... DROP COLUMN` | only when an index or constraint still pins the column; otherwise it is emitted, dropping blocking indexes first |
+| `ALTER TABLE ... DROP COLUMN` | emitted after dropping blocking indexes when no index or constraint still references the column; otherwise the statement is dropped |

This follows the stated converter behavior in the line-range change details.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
| `ALTER TABLE ... DROP COLUMN` | only when an index or constraint still pins the column; otherwise it is emitted, dropping blocking indexes first |
| `ALTER TABLE ... DROP COLUMN` | emitted after dropping blocking indexes when no index or constraint still references the column; otherwise the statement is dropped |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/databases.md` at line 86, Correct the `ALTER TABLE ... DROP COLUMN`
description to state that the converter emits the statement after removing
blocking indexes when no index or constraint still references the column; when a
reference remains, it should omit the statement.

| `CREATE STATISTICS`, publications, RLS policies | no planner statistics or replication objects |

The practical consequence: **a SQLite database has weaker integrity guarantees
Expand Down Expand Up @@ -128,6 +128,37 @@ correct, coverage is narrower. Stemming, ranking and multi-word queries are lost
connection layer skips these on SQLite; concurrency is serialised by SQLite's
single-writer model instead.

## Postgres-only SQL in query code

Migrations are not the only place Postgres dialect leaks in. Query code can hardcode
it too, and that only fails when the code path runs — the RBAC role lookup broke login
on SQLite this way, long after migrations and seeding both passed.

The current-timestamp idiom is the common case and now has a helper:

```go
r.db.NowEpoch() // on a *postgres.Connection
dbdialect.NowEpochFromBun(db) // when you only have a bun.IDB or a tx
```

`TestNoHardcodedEpochExpression` in `pkg/dbdialect` fails the build if
`extract(epoch ...)` reappears in query SQL. Model `default:` tags are exempt: bun
only emits those for zero values, and `BeforeAppendModel` sets the fields first.

A file that is deliberately Postgres-only opts out with a `dialect:postgres-only`
marker near its package clause, and must gate itself on a capability instead.

### Known gaps not yet addressed

These still contain Postgres-only SQL and will fail at runtime on SQLite:

| Construct | Extent | Note |
|---|---|---|
| `ILIKE` | 17 files | SQLite `LIKE` is already case-insensitive for ASCII, so this is a mechanical swap |
| `::int`, `::float`, `::bigint`, `::numeric`, `::text`, `::jsonb` casts | ~109 occurrences | mostly in analytics and reporting aggregates |
| `date_trunc` bucketing | reporting compiler, A/R analytics | marked `dialect:postgres-only`; needs a `strftime` equivalent |
| `pg_stat_activity`, `pg_blocking_pids` | database session diagnostics | gated on `CapSessionDiagnostic`, returns 501 on SQLite |

## Error handling

`pkg/dberror` maps SQLite result codes onto the SQLSTATE codes the repositories
Expand Down
7 changes: 5 additions & 2 deletions services/tms/internal/core/services/ediservice/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/emoss08/trenova/internal/core/services/internaledilifecycle"
"github.com/emoss08/trenova/internal/core/services/notificationservice"
"github.com/emoss08/trenova/internal/infrastructure/observability/metrics"
"github.com/emoss08/trenova/pkg/dbdialect"
"github.com/emoss08/trenova/pkg/dberror"
"github.com/emoss08/trenova/pkg/domaintypes"
"github.com/emoss08/trenova/pkg/errortypes"
Expand Down Expand Up @@ -1812,12 +1813,14 @@ func (s *Service) setShipmentTenderStatus(
tenantInfo pagination.TenantInfo,
status shipment.TenderStatus,
) error {
results, err := s.db.DBForContext(ctx).
db := s.db.DBForContext(ctx)

results, err := db.
NewUpdate().
Model((*shipment.Shipment)(nil)).
Set("tender_status = ?", status).
Set("version = version + 1").
Set("updated_at = extract(epoch from current_timestamp)::bigint").
Set("updated_at = "+dbdialect.NowEpochFromBun(db)).
Where("id = ?", shipmentID).
Where("organization_id = ?", tenantInfo.OrgID).
Where("business_unit_id = ?", tenantInfo.BuID).
Expand Down
17 changes: 11 additions & 6 deletions services/tms/internal/core/services/hosprojection/project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ func TestProject_PlentyOfHoursRunsOnCurrentClocks(t *testing.T) {
t.Parallel()

result := Project(Input{
Now: baseTime,
Departure: baseTime,
Now: baseTime,
Departure: baseTime,
TripDriveMs: hoursMs(4),
Clocks: Clocks{
DriveMs: hoursMs(9),
Expand Down Expand Up @@ -295,10 +295,15 @@ func TestProject_CanadianJurisdictionSkipsSplitAndRestart(t *testing.T) {
BreakMs: 8 * hourMs,
}
result := Project(Input{
Now: baseTime,
Departure: baseTime + hoursSec(40),
TripDriveMs: hoursMs(8),
Clocks: Clocks{DriveMs: hoursMs(8), ShiftMs: hoursMs(10), CycleMs: hoursMs(1), BreakMs: hoursMs(8)},
Now: baseTime,
Departure: baseTime + hoursSec(40),
TripDriveMs: hoursMs(8),
Clocks: Clocks{
DriveMs: hoursMs(8),
ShiftMs: hoursMs(10),
CycleMs: hoursMs(1),
BreakMs: hoursMs(8),
},
ClocksAt: baseTime,
DutyStatus: telematics.DutyStatusOffDuty,
Limits: limits,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// dialect:postgres-only — report date bucketing compiles to date_trunc.
package compiler

import (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/emoss08/trenova/internal/core/domain/worker"
"github.com/emoss08/trenova/internal/core/services/driversettlementservice"
"github.com/emoss08/trenova/internal/infrastructure/database/common"
"github.com/emoss08/trenova/pkg/dbdialect"
"github.com/emoss08/trenova/pkg/domaintypes"
"github.com/emoss08/trenova/pkg/seedhelpers"
"github.com/emoss08/trenova/shared/pulid"
Expand Down Expand Up @@ -73,19 +74,40 @@ func (s *DriverPayLedgerSeed) Run(ctx context.Context, tx bun.Tx) error {
return fmt.Errorf("ensure ledger accounts: %w", err)
}

if err = s.configureAccountingControl(ctx, tx, org.ID, org.BusinessUnitID, accounts); err != nil {
if err = s.configureAccountingControl(
ctx,
tx,
org.ID,
org.BusinessUnitID,
accounts,
); err != nil {
return fmt.Errorf("configure accounting control: %w", err)
}

if err = s.configureCostCategories(ctx, tx, org.ID, org.BusinessUnitID); err != nil {
return fmt.Errorf("configure cost categories: %w", err)
}

if err = s.ensureRecurringEarnings(ctx, tx, sc, org.ID, org.BusinessUnitID, admin.ID); err != nil {
if err = s.ensureRecurringEarnings(
ctx,
tx,
sc,
org.ID,
org.BusinessUnitID,
admin.ID,
); err != nil {
return fmt.Errorf("ensure recurring earnings: %w", err)
}

if err = s.postSeededSettlementJournals(ctx, tx, sc, org.ID, org.BusinessUnitID, admin.ID, accounts); err != nil {
if err = s.postSeededSettlementJournals(
ctx,
tx,
sc,
org.ID,
org.BusinessUnitID,
admin.ID,
accounts,
); err != nil {
return fmt.Errorf("post seeded settlement journals: %w", err)
}

Expand Down Expand Up @@ -322,14 +344,86 @@ func (s *DriverPayLedgerSeed) ensureRecurringEarnings(
capMinor int64
paidMinor int64
}{
{payWorkerJohn, "PERDIEM", driverpay.EarningStatusActive, driverpay.EarningFrequencyEverySettlement, "OTR per diem — IRS substantiated M&IE", 33250, 0, 299250},
{payWorkerRobert, "PERDIEM", driverpay.EarningStatusActive, driverpay.EarningFrequencyEverySettlement, "OTR per diem — IRS substantiated M&IE", 33250, 0, 199500},
{payWorkerEmily, "PERDIEM", driverpay.EarningStatusActive, driverpay.EarningFrequencyEverySettlement, "Regional per diem — partial-day M&IE", 19950, 0, 119700},
{payWorkerJane, "SAFETY", driverpay.EarningStatusActive, driverpay.EarningFrequencyMonthly, "Quarterly safety bonus accrual — clean CSA record", 12500, 0, 62500},
{payWorkerSarah, "STIPEND", driverpay.EarningStatusActive, driverpay.EarningFrequencyMonthly, "Cell phone and ELD data stipend", 5000, 0, 25000},
{payWorkerMike, "PERFORM", driverpay.EarningStatusActive, driverpay.EarningFrequencyEverySettlement, "On-time delivery bonus program", 7500, 195000, 97500},
{payWorkerCarlos, "EQUIPRENT", driverpay.EarningStatusActive, driverpay.EarningFrequencyEverySettlement, "APU rental — company use of driver-owned unit", 6000, 0, 78000},
{payWorkerDavid, "LONGEVITY", driverpay.EarningStatusPaused, driverpay.EarningFrequencyMonthly, "Longevity bonus — 3+ years of service", 10000, 0, 30000},
{
payWorkerJohn,
"PERDIEM",
driverpay.EarningStatusActive,
driverpay.EarningFrequencyEverySettlement,
"OTR per diem — IRS substantiated M&IE",
33250,
0,
299250,
},
{
payWorkerRobert,
"PERDIEM",
driverpay.EarningStatusActive,
driverpay.EarningFrequencyEverySettlement,
"OTR per diem — IRS substantiated M&IE",
33250,
0,
199500,
},
{
payWorkerEmily,
"PERDIEM",
driverpay.EarningStatusActive,
driverpay.EarningFrequencyEverySettlement,
"Regional per diem — partial-day M&IE",
19950,
0,
119700,
},
{
payWorkerJane,
"SAFETY",
driverpay.EarningStatusActive,
driverpay.EarningFrequencyMonthly,
"Quarterly safety bonus accrual — clean CSA record",
12500,
0,
62500,
},
{
payWorkerSarah,
"STIPEND",
driverpay.EarningStatusActive,
driverpay.EarningFrequencyMonthly,
"Cell phone and ELD data stipend",
5000,
0,
25000,
},
{
payWorkerMike,
"PERFORM",
driverpay.EarningStatusActive,
driverpay.EarningFrequencyEverySettlement,
"On-time delivery bonus program",
7500,
195000,
97500,
},
{
payWorkerCarlos,
"EQUIPRENT",
driverpay.EarningStatusActive,
driverpay.EarningFrequencyEverySettlement,
"APU rental — company use of driver-owned unit",
6000,
0,
78000,
},
{
payWorkerDavid,
"LONGEVITY",
driverpay.EarningStatusPaused,
driverpay.EarningFrequencyMonthly,
"Longevity bonus — 3+ years of service",
10000,
0,
30000,
},
}

rows := make([]*driverpay.RecurringEarning, 0, len(defs))
Expand Down Expand Up @@ -375,7 +469,10 @@ func (s *DriverPayLedgerSeed) ensureRecurringEarnings(

seedhelpers.LogSuccess(
"Created recurring earning fixtures",
fmt.Sprintf("- Created %d recurring earnings (per diem, bonuses, stipends, equipment rental)", len(rows)),
fmt.Sprintf(
"- Created %d recurring earnings (per diem, bonuses, stipends, equipment rental)",
len(rows),
),
)
return nil
}
Expand Down Expand Up @@ -698,7 +795,10 @@ func (s *DriverPayLedgerSeed) postSeededSettlementJournals(
if posted > 0 {
seedhelpers.LogSuccess(
"Posted seeded settlements to the general ledger",
fmt.Sprintf("- Created %d balanced journal batches with period balances for cost control", posted),
fmt.Sprintf(
"- Created %d balanced journal batches with period balances for cost control",
posted,
),
)
}
return nil
Expand Down Expand Up @@ -728,7 +828,7 @@ func (s *DriverPayLedgerSeed) applyBalances(
period_credit_minor = gb.period_credit_minor + EXCLUDED.period_credit_minor,
net_change_minor = gb.net_change_minor + EXCLUDED.net_change_minor,
last_journal_entry_id = EXCLUDED.last_journal_entry_id,
updated_at = extract(epoch from current_timestamp)::bigint
updated_at = `+dbdialect.NowEpochFromBun(tx)+`
`,
orgID, buID, line.GLAccountID, fiscalYearID, fiscalPeriodID,
line.DebitAmount, line.CreditAmount, line.NetAmount, entryID,
Expand All @@ -740,7 +840,7 @@ func (s *DriverPayLedgerSeed) applyBalances(
Set("current_balance = current_balance + ?", line.NetAmount).
Set("debit_balance = debit_balance + ?", line.DebitAmount).
Set("credit_balance = credit_balance + ?", line.CreditAmount).
Set("updated_at = extract(epoch from current_timestamp)::bigint").
Set("updated_at = "+dbdialect.NowEpochFromBun(tx)).
Where("id = ?", line.GLAccountID).
Where("organization_id = ?", orgID).
Where("business_unit_id = ?", buID).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import (
"testing"
"time"

fitz "github.com/gen2brain/go-fitz"
"github.com/emoss08/trenova/internal/core/domain/documenttemplate"
"github.com/emoss08/trenova/internal/core/ports/services"
"github.com/emoss08/trenova/internal/infrastructure/config"
fitz "github.com/gen2brain/go-fitz"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
Expand Down
41 changes: 34 additions & 7 deletions services/tms/internal/infrastructure/postgres/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,18 @@ func oltpSettings(cfg *config.Config) connectionSettings {
connMaxLifetime: cfg.Database.ConnMaxLifetime,
connMaxIdleTime: cfg.Database.ConnMaxIdleTime,
connParams: map[string]any{
"statement_timeout": fmt.Sprintf("%dms", max(cfg.Database.GetStatementTimeout().Milliseconds(), 1)),
"lock_timeout": fmt.Sprintf("%dms", max(cfg.Database.GetLockTimeout().Milliseconds(), 1)),
"idle_in_transaction_session_timeout": fmt.Sprintf("%dms", max(cfg.Database.GetIdleTxTimeout().Milliseconds(), 1)),
"statement_timeout": fmt.Sprintf(
"%dms",
max(cfg.Database.GetStatementTimeout().Milliseconds(), 1),
),
"lock_timeout": fmt.Sprintf(
"%dms",
max(cfg.Database.GetLockTimeout().Milliseconds(), 1),
),
"idle_in_transaction_session_timeout": fmt.Sprintf(
"%dms",
max(cfg.Database.GetIdleTxTimeout().Milliseconds(), 1),
),
},
registerStats: true,
}
Expand All @@ -130,10 +139,19 @@ func reportingSettings(cfg *config.Config) connectionSettings {
connMaxLifetime: cfg.Database.ConnMaxLifetime,
connMaxIdleTime: cfg.Database.ConnMaxIdleTime,
connParams: map[string]any{
"statement_timeout": fmt.Sprintf("%dms", max(reporting.GetStatementTimeout().Milliseconds(), 1)),
"lock_timeout": fmt.Sprintf("%dms", max(cfg.Database.GetLockTimeout().Milliseconds(), 1)),
"idle_in_transaction_session_timeout": fmt.Sprintf("%dms", max(cfg.Database.GetIdleTxTimeout().Milliseconds(), 1)),
"default_transaction_read_only": "on",
"statement_timeout": fmt.Sprintf(
"%dms",
max(reporting.GetStatementTimeout().Milliseconds(), 1),
),
"lock_timeout": fmt.Sprintf(
"%dms",
max(cfg.Database.GetLockTimeout().Milliseconds(), 1),
),
"idle_in_transaction_session_timeout": fmt.Sprintf(
"%dms",
max(cfg.Database.GetIdleTxTimeout().Milliseconds(), 1),
),
"default_transaction_read_only": "on",
},
registerStats: false,
}
Expand Down Expand Up @@ -382,3 +400,12 @@ func (c *Connection) Close() error {
}
return nil
}

// NowEpoch is the current-Unix-timestamp SQL for the connection's dialect.
func (c *Connection) NowEpoch() string {
if c.cfg == nil {
return dbdialect.DefaultKind.NowEpoch()
}

return c.cfg.Database.GetDialect().NowEpoch()
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// dialect:postgres-only — collections analytics bucket dates with date_trunc.
package accountsreceivablerepository

import (
Expand Down
Loading
Loading