From 21e01cc1e61757e4ecdb320cff79801a5e17f07e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rouzbeh=E2=80=A0?= <78313022+rqzbeh@users.noreply.github.com> Date: Mon, 8 Jun 2026 14:54:53 +0200 Subject: [PATCH] fix(postgres): make node traffic sync robust after public API inbound updates (#5038) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(postgres): make node traffic sync robust after public API inbound updates The background NodeTrafficSyncJob (every 5s) started failing after a successful POST /panel/api/inbounds/update/{id} (including flows that inject streamSettings.externalProxy) with: node traffic sync: merge for failed: ERROR: CASE types boolean and integer cannot be matched (SQLSTATE 42804) Root cause: - The merge lives in setRemoteTrafficLocked (called from SetRemoteTraffic). - The client_traffics delta path used a dialect-sensitive expression: enable = enable AND ? last_online = GREATEST(last_online, ?) - On PostgreSQL, GREATEST / AND / COALESCE are implemented with internal CASE expressions. When "enable" columns (client_traffics, inbounds, ...) were INTEGER (common after SQLite → PG data migrations, older AutoMigrate, or mixed write paths) and the right-hand side was a boolean parameter (from snapshot ClientStats or form-bound API payload), PG rejected the expression at plan time. - The public API update path (unlike the internal remote wire path) always runs updateClientTraffics + UpdateClientStat + SyncInbound. This touches client_traffics.enable rows for any inbound that has clients. - SQLite tolerated 0/1 numeric bools; PG is strict. Fix: - Use an explicit CASE with ::boolean casts in the critical enable expression so the result type is always boolean. - Make GreatestExpr emit safe casts on Postgres. - Add a one-time normalization step in MigrationRequirements (runs on startup + xray restarts) that forces the relevant enable/enabled columns to boolean on Postgres using an idempotent DO block + USING cast. This cleans up pre-existing skew without a full re-migration. This branch is based on upstream/main (original mhsanaei/3x-ui main). The node traffic sync now survives arbitrary public-API inbound updates on PostgreSQL. * fix: make client traffic enable merge expression safe on SQLite too The previous commit introduced an explicit CASE for the "only node can disable" logic in the node traffic sync merge to fix the PG "CASE types boolean and integer cannot be matched" error after public API inbound updates. That expression used PostgreSQL-only `::boolean` casts: CASE WHEN ?::boolean THEN enable::boolean ELSE false END This is invalid syntax on SQLite (and would break the merge when the client_traffics delta UPDATE runs — which is commonly triggered right after an API /inbounds/update because that path calls updateClientTraffics + SyncInbound and touches client_traffics rows). Extracted the expression to a new dialect-aware helper `ClientTrafficEnableMergeExpr()` (following the same pattern as GreatestExpr, JSONClientsFromInbound, etc.). - On Postgres: keeps the strict boolean-typed CASE with casts. - On SQLite: uses a numeric-compatible form `CASE WHEN ? THEN enable ELSE 0 END` that produces the expected 0/1 result matching the column affinity. The logical behavior ("node may only force-disable, never re-enable") is preserved on both databases. This is a follow-up commit on the same branch so that one PR contains both the original Postgres fix and the SQLite compatibility fix. Builds directly on top of 91643f68. * fix --------- Co-authored-by: Rqzbeh Co-authored-by: Sanaei --- database/dialect.go | 26 ++++++++++++++++++++++++- web/service/inbound.go | 43 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/database/dialect.go b/database/dialect.go index bdf4486c..48aa2de2 100644 --- a/database/dialect.go +++ b/database/dialect.go @@ -22,7 +22,31 @@ func JSONFieldText(expr, key string) string { func GreatestExpr(a, b string) string { if IsPostgres() { - return fmt.Sprintf("GREATEST(%s, %s)", a, b) + return fmt.Sprintf("GREATEST(%s::bigint, %s::bigint)", a, b) } return fmt.Sprintf("MAX(%s, %s)", a, b) } + +// ClientTrafficEnableMergeExpr returns the SQL expression used in the +// node traffic merge to update client_traffics.enable. +// +// The intent is: only allow the remote node to *disable* a client +// (never re-enable one that the central panel has disabled). +// +// We use a dialect-specific expression because: +// - On PostgreSQL we want strict boolean typing and casts to avoid +// "CASE types boolean and integer cannot be matched" errors +// (and similar internal expansions of AND/GREATEST). +// - On SQLite, enable is stored with INTEGER affinity (0/1), there is +// no :: cast syntax, and we must produce a numeric-compatible result. +// +// The expression must be valid SQL for tx.Exec with a boolean parameter +// as the first ?. +func ClientTrafficEnableMergeExpr() string { + if IsPostgres() { + return "CASE WHEN ?::boolean THEN enable::boolean ELSE false END" + } + // SQLite: no :: casts. Use numeric CASE. 1/0 work as true/false + // thanks to SQLite's affinity and how GORM/drivers bind bools. + return "CASE WHEN ? THEN enable ELSE 0 END" +} diff --git a/web/service/inbound.go b/web/service/inbound.go index 0e70cb95..c5c5c8ac 100644 --- a/web/service/inbound.go +++ b/web/service/inbound.go @@ -1841,7 +1841,14 @@ func (s *InboundService) setRemoteTrafficLocked(nodeID int, snap *runtime.Traffi // from the node arriving after a central disable would otherwise // overwrite enable=false back to true, letting the client accumulate // far more traffic than their limit before being disabled again. - enableExpr := "enable AND ?" + // + // We use a dialect-aware expression (see database.ClientTrafficEnableMergeExpr) + // because the old "enable AND ?" form (and naive CASE with :: casts) + // caused type mismatches on PostgreSQL after public API inbound updates + // (which go through updateClientTraffics + SyncInbound and can touch + // client_traffics rows) and would also break on SQLite due to PG-only + // ::boolean syntax. + enableExpr := database.ClientTrafficEnableMergeExpr() if err := tx.Exec( fmt.Sprintf( `UPDATE client_traffics @@ -3556,6 +3563,40 @@ func (s *InboundService) MigrationRequirements() { } } + // Normalize "enable" columns to boolean on Postgres. Legacy SQLite data + // (0/1 integers), partial migrations, or mixed write paths (public API + // inbound updates that flow through UpdateClientStat + client syncs, plus + // node traffic merge deltas) can leave the column as integer or with mixed + // interpretation. This (combined with the dialect-aware + // ClientTrafficEnableMergeExpr) prevents type problems in the node traffic + // sync merge (SetRemoteTraffic) and makes the sync robust even when + // inbounds are updated via the public API (incl. ones carrying + // externalProxy in streamSettings). The same expression is also safe on + // SQLite (no PG :: casts). + if database.IsPostgres() { + // Use DO block so it is idempotent and doesn't fail if already boolean. + normalizeBool := func(table, col string) { + tx.Exec(fmt.Sprintf(` + DO $$ + BEGIN + IF EXISTS ( + SELECT 1 FROM information_schema.columns + WHERE table_name = '%s' AND column_name = '%s' + AND data_type <> 'boolean' + ) THEN + ALTER TABLE %s ALTER COLUMN %s + TYPE boolean USING (CASE WHEN %s::text IN ('1','true','t','yes') THEN true ELSE false END); + END IF; + END $$;`, table, col, table, col, col)) + } + normalizeBool("inbounds", "enable") + normalizeBool("client_traffics", "enable") + normalizeBool("nodes", "enable") + normalizeBool("clients", "enable") + normalizeBool("api_tokens", "enabled") + normalizeBool("outbound_subscriptions", "enabled") + } + // Fix inbounds based problems var inbounds []*model.Inbound err = tx.Model(model.Inbound{}).Where("protocol IN (?)", []string{"vmess", "vless", "trojan", "shadowsocks", "hysteria"}).Find(&inbounds).Error