diff --git a/_FiveSql2/src/TSqlExecutor.prg b/_FiveSql2/src/TSqlExecutor.prg index 0d87fe4..25a9141 100644 --- a/_FiveSql2/src/TSqlExecutor.prg +++ b/_FiveSql2/src/TSqlExecutor.prg @@ -26,8 +26,15 @@ * guards (TSqlAutoInc.go / SqlBumpSchemaVer). */ -STATIC s_hAutoInc := NIL -STATIC s_nRCJSeq := 0 +/* s_hAutoInc / s_nRCJSeq / s_nSchemaVer used to live here as + * module STATICs. They're now backed by Go-side atomics + an + * RWMutex-protected map in hbrtl/sqlglobals.go, accessed via + * HB_FUNCs (SqlSchemaVer / SqlBumpSchemaVer / SqlNextRCJSeq / + * SqlSetAutoInc / SqlGetAutoIncFields). Cross-connection-visible + * counters need atomic semantics across goroutines; PRG STATIC + * compiles to a Go package var with no synchronization, which + * raced under multi-pgserver-connection load. + */ /* Per-plan DML pcode cache. Keyed by the plan-cache key that TFiveSQL * uses (template key or cSQL text); value is a hash: @@ -69,7 +76,7 @@ RETURN NIL * * STATIC is file-scoped, so cross-file access goes through the * SqlSchemaVer / SqlBumpSchemaVer top-level functions below. */ -STATIC s_nSchemaVer := 0 +/* s_nSchemaVer — see Go-side hbrtl/sqlglobals.go */ CLASS TSqlExecutor @@ -4495,36 +4502,23 @@ FUNCTION SqlMaterializeSubquery( xSubQ, cAlias, aParams ) RETURN { cTmpFile, cAlias, "" } -/* Auto-increment support */ -FUNCTION SqlSetAutoInc( cTable, cField ) - - LOCAL cKey - - cKey := Upper( cTable ) - IF s_hAutoInc == NIL - s_hAutoInc := { => } - ENDIF - IF ! hb_HHasKey( s_hAutoInc, cKey ) - s_hAutoInc[ cKey ] := {} - ENDIF - AAdd( s_hAutoInc[ cKey ], Upper( cField ) ) - -RETURN NIL - -FUNCTION SqlGetAutoIncFields( cTable ) - - LOCAL cKey - - cKey := Upper( cTable ) - IF s_hAutoInc == NIL - RETURN {} - ENDIF - IF hb_HHasKey( s_hAutoInc, cKey ) - RETURN s_hAutoInc[ cKey ] - ENDIF - -RETURN {} - +/* SqlSetAutoInc / SqlGetAutoIncFields used to live here as PRG + * helpers around the s_hAutoInc STATIC. They're now Go HB_FUNCs + * backed by an RWMutex-protected map (hbrtl/sqlglobals.go); + * PRG callers see the same signature and semantics, just thread- + * safe under concurrent CREATE TABLE + INSERT. The old PRG + * implementation below is dead, kept commented for one release + * cycle so any porter looking for `SqlSetAutoInc` can grep it. + * + * STATIC s_hAutoInc := NIL — gone (Go-side now) + * + * FUNCTION SqlSetAutoInc(cTable, cField) — Go HB_FUNC + * FUNCTION SqlGetAutoIncFields(cTable) — Go HB_FUNC + * + * The two-line guard branch the original maintained to compare + * against NIL is replaced by Go's `if v, ok := m[k]; ok {...}` + * pattern in hbSqlGetAutoIncFields — same semantics, no per-call + * NIL-check on the PRG side. */ FUNCTION SqlGetMaxFieldVal( cAlias, cField ) LOCAL nWA, nSaved, nFPos, nMax := 0, xVal @@ -5610,8 +5604,7 @@ STATIC FUNCTION RecCteJoin( hRecQuery, aFN, aPrevRows, cCteName ) cDbfFile := cDbfFile + ".dbf" ENDIF - s_nRCJSeq := ( s_nRCJSeq + 1 ) % 100000 - cWAAlias := "RCJ_" + hb_ntos( s_nRCJSeq ) + cWAAlias := "RCJ_" + hb_ntos( SqlNextRCJSeq() ) BEGIN SEQUENCE USE ( cDbfFile ) NEW SHARED ALIAS ( cWAAlias ) @@ -6208,10 +6201,7 @@ RETURN NIL * schema. Called from TFiveSQL (plan cache key build) and * TSqlDDL (invalidation after CREATE/ALTER/DROP). * -------------------------------------------------------------- */ -FUNCTION SqlSchemaVer() -RETURN s_nSchemaVer - - -FUNCTION SqlBumpSchemaVer() - s_nSchemaVer++ -RETURN s_nSchemaVer +/* SqlSchemaVer / SqlBumpSchemaVer are Go HB_FUNCs (atomic.uint64) + * — see hbrtl/sqlglobals.go. The PRG-level definitions were + * removed: a STATIC int32 with non-atomic ++ couldn't safely + * serialise concurrent DDL across pgserver connections. */ diff --git a/hbrtl/sqlglobals.go b/hbrtl/sqlglobals.go new file mode 100644 index 0000000..64e6b9f --- /dev/null +++ b/hbrtl/sqlglobals.go @@ -0,0 +1,116 @@ +// Copyright (c) 2026 Charles KWON OhJun (charleskwonohjun@gmail.com) +// All rights reserved. + +// sqlglobals.go — Process-wide FiveSql2 counters that genuinely need +// cross-connection visibility, exposed to PRG via HB_FUNCs that wrap +// atomic / mutex primitives on the Go side. The PRG STATICs they +// replace (s_nSchemaVer, s_nRCJSeq, s_hAutoInc in TSqlExecutor.prg) +// were Go package vars under gengo; multi-pgserver-connection use +// raced on them and produced wrong cache invalidation, duplicate +// temp aliases, and "concurrent map writes" panics on the AUTOINC +// map's lazy first-init path. +// +// Wrapping these in Go-level synchronization is cheaper than the +// alternative (move to TSqlSession + thread oSession through every +// DDL caller in TSqlDDL.prg + IDENTITY handler in TSqlExecutor.prg) +// AND gives the right semantic — a CREATE TABLE on connection A +// MUST invalidate connection B's plan cache, an IDENTITY column +// MUST hand out monotonic values across all writers. + +package hbrtl + +import ( + "strings" + "sync" + "sync/atomic" + + "five/hbrt" +) + +// --- Schema version counter --- +// +// Every DDL statement (CREATE / ALTER / DROP TABLE / INDEX / VIEW) +// calls SQL_BUMP_SCHEMA_VER. Plan-cache + DML-pcode-cache keys embed +// the current value as a prefix, so a bump invalidates every cached +// entry across every connection in one atomic write. +var sqlSchemaVer uint64 + +// --- RCJ sequence counter --- +// +// Recursive Common Join helper temp-alias counter. Each +// `RCJ_NNN` alias needs cross-connection uniqueness because two +// concurrent queries materialising different CTEs would otherwise +// collide on RCJ_1. Mod 100000 to keep aliases short. +var sqlRCJSeq uint64 + +// --- IDENTITY column registry --- +// +// Maps tableName (upper) → list of IDENTITY field names (upper). +// Populated by CREATE TABLE; read by INSERT to compute next value. +// Genuinely process-global: two connections inserting into the +// same table must see the same IDENTITY-field set. +var ( + sqlAutoIncMu sync.RWMutex + sqlAutoInc = map[string][]string{} +) + +func init() { + hbrt.HB_FUNC("SQLSCHEMAVER", hbSqlSchemaVer) + hbrt.HB_FUNC("SQLBUMPSCHEMAVER", hbSqlBumpSchemaVer) + hbrt.HB_FUNC("SQLNEXTRCJSEQ", hbSqlNextRCJSeq) + hbrt.HB_FUNC("SQLSETAUTOINC", hbSqlSetAutoInc) + hbrt.HB_FUNC("SQLGETAUTOINCFIELDS", hbSqlGetAutoIncFields) +} + +func hbSqlSchemaVer(ctx *hbrt.HBContext) { + ctx.RetNI(int(atomic.LoadUint64(&sqlSchemaVer))) +} + +func hbSqlBumpSchemaVer(ctx *hbrt.HBContext) { + ctx.RetNI(int(atomic.AddUint64(&sqlSchemaVer, 1))) +} + +// hbSqlNextRCJSeq returns the next (atomic) RCJ sequence value +// modulo 100000, matching the previous PRG `s_nRCJSeq := (s_nRCJSeq + 1) % 100000` +// behaviour. Atomic increment first, then mod for the user-facing value. +func hbSqlNextRCJSeq(ctx *hbrt.HBContext) { + n := atomic.AddUint64(&sqlRCJSeq, 1) + ctx.RetNI(int(n % 100000)) +} + +// hbSqlSetAutoInc registers a field as IDENTITY for a table. +// Replaces TSqlExecutor.prg's SqlSetAutoInc which lazily init'd +// a STATIC hash → "concurrent map writes" panic under two +// concurrent CREATE TABLE calls. +func hbSqlSetAutoInc(ctx *hbrt.HBContext) { + if ctx.PCount() < 2 || !ctx.IsChar(1) || !ctx.IsChar(2) { + ctx.RetNil() + return + } + key := strings.ToUpper(ctx.ParC(1)) + field := strings.ToUpper(ctx.ParC(2)) + sqlAutoIncMu.Lock() + defer sqlAutoIncMu.Unlock() + sqlAutoInc[key] = append(sqlAutoInc[key], field) + ctx.RetNil() +} + +// hbSqlGetAutoIncFields returns the list of IDENTITY field names +// for a table, or an empty array if none. PRG signature returns +// an Array of strings. +func hbSqlGetAutoIncFields(ctx *hbrt.HBContext) { + if ctx.PCount() < 1 || !ctx.IsChar(1) { + arr := hbrt.MakeArrayFrom(nil) + ctx.RetVal(arr) + return + } + key := strings.ToUpper(ctx.ParC(1)) + sqlAutoIncMu.RLock() + fields := sqlAutoInc[key] + sqlAutoIncMu.RUnlock() + items := make([]hbrt.Value, len(fields)) + for i, f := range fields { + items[i] = hbrt.MakeString(f) + } + ctx.RetVal(hbrt.MakeArrayFrom(items)) +}