Skip to content

Commit 023fa6d

Browse files
lingcoderlingcoder
committed
fix(pgsql,gaussdb): capture INSERT...RETURNING via CatchSQL/ToSQL
DoExec for InsertAndGetId manually calls FormatSqlBeforeExecuting, DoFilter and DoCommit, bypassing Core.DoQuery's CatchSQLManager handling. This caused: - CatchSQL silently drops the RETURNING SQL (never appended to SQLArray) - ToSQL ignores DoCommit=false and executes the INSERT anyway Delegate to Core.DoQuery so both capture and short-circuit behaviors are honored uniformly. Also tighten Test_Issue3204 assertion and add two dedicated regression tests for CatchSQL and ToSQL on InsertAndGetId.
1 parent cd579f8 commit 023fa6d

4 files changed

Lines changed: 84 additions & 54 deletions

File tree

‎contrib/drivers/gaussdb/gaussdb_do_exec.go‎

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -62,30 +62,15 @@ func (d *Driver) DoExec(ctx context.Context, link gdb.Link, sql string, args ...
6262
return d.Core.DoExec(ctx, link, sql, args...)
6363
}
6464

65-
// Only the insert operation with primary key can execute the following code
66-
67-
// Sql filtering.
68-
sql, args = d.FormatSqlBeforeExecuting(sql, args)
69-
sql, args, err = d.DoFilter(ctx, link, sql, args)
70-
if err != nil {
71-
return nil, err
72-
}
73-
74-
// Link execution.
75-
var out gdb.DoCommitOutput
76-
out, err = d.DoCommit(ctx, gdb.DoCommitInput{
77-
Link: link,
78-
Sql: sql,
79-
Args: args,
80-
Stmt: nil,
81-
Type: gdb.SqlTypeQueryContext,
82-
IsTransaction: link.IsTransaction(),
83-
})
84-
65+
// Only the insert operation with primary key can execute the following code.
66+
// Delegate to Core.DoQuery so it handles FormatSqlBeforeExecuting, DoFilter,
67+
// CatchSQL capture, and DoCommit uniformly — avoids silently dropping the SQL
68+
// from CatchSQLManager when RETURNING is appended.
69+
records, err := d.Core.DoQuery(ctx, link, sql, args...)
8570
if err != nil {
8671
return nil, err
8772
}
88-
affected := len(out.Records)
73+
affected := len(records)
8974
if affected > 0 {
9075
if !strings.Contains(pkField.Type, "int") {
9176
return Result{
@@ -97,8 +82,8 @@ func (d *Driver) DoExec(ctx context.Context, link gdb.Link, sql string, args ...
9782
}, nil
9883
}
9984

100-
if out.Records[affected-1][primaryKey] != nil {
101-
lastInsertId := out.Records[affected-1][primaryKey].Int64()
85+
if records[affected-1][primaryKey] != nil {
86+
lastInsertId := records[affected-1][primaryKey].Int64()
10287
return Result{
10388
affected: int64(affected),
10489
lastInsertId: lastInsertId,

‎contrib/drivers/pgsql/pgsql_do_exec.go‎

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -62,30 +62,15 @@ func (d *Driver) DoExec(ctx context.Context, link gdb.Link, sql string, args ...
6262
return d.Core.DoExec(ctx, link, sql, args...)
6363
}
6464

65-
// Only the insert operation with primary key can execute the following code
66-
67-
// Sql filtering.
68-
sql, args = d.FormatSqlBeforeExecuting(sql, args)
69-
sql, args, err = d.DoFilter(ctx, link, sql, args)
70-
if err != nil {
71-
return nil, err
72-
}
73-
74-
// Link execution.
75-
var out gdb.DoCommitOutput
76-
out, err = d.DoCommit(ctx, gdb.DoCommitInput{
77-
Link: link,
78-
Sql: sql,
79-
Args: args,
80-
Stmt: nil,
81-
Type: gdb.SqlTypeQueryContext,
82-
IsTransaction: link.IsTransaction(),
83-
})
84-
65+
// Only the insert operation with primary key can execute the following code.
66+
// Delegate to Core.DoQuery so it handles FormatSqlBeforeExecuting, DoFilter,
67+
// CatchSQL capture, and DoCommit uniformly — avoids silently dropping the SQL
68+
// from CatchSQLManager when RETURNING is appended.
69+
records, err := d.Core.DoQuery(ctx, link, sql, args...)
8570
if err != nil {
8671
return nil, err
8772
}
88-
affected := len(out.Records)
73+
affected := len(records)
8974
if affected > 0 {
9075
if !strings.Contains(pkField.Type, "int") {
9176
return Result{
@@ -97,8 +82,8 @@ func (d *Driver) DoExec(ctx context.Context, link gdb.Link, sql string, args ...
9782
}, nil
9883
}
9984

100-
if out.Records[affected-1][primaryKey] != nil {
101-
lastInsertId := out.Records[affected-1][primaryKey].Int64()
85+
if records[affected-1][primaryKey] != nil {
86+
lastInsertId := records[affected-1][primaryKey].Int64()
10287
return Result{
10388
affected: int64(affected),
10489
lastInsertId: lastInsertId,

‎contrib/drivers/pgsql/pgsql_z_unit_feature_pgsql_test.go‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,15 @@
77
package pgsql_test
88

99
import (
10+
"context"
1011
"fmt"
1112
"testing"
1213

14+
"github.com/gogf/gf/v2/database/gdb"
1315
"github.com/gogf/gf/v2/frame/g"
1416
"github.com/gogf/gf/v2/os/gtime"
1517
"github.com/gogf/gf/v2/test/gtest"
18+
"github.com/gogf/gf/v2/text/gstr"
1619
)
1720

1821
// =============================================================================
@@ -359,6 +362,67 @@ func Test_PgSQL_Returning_Upsert(t *testing.T) {
359362
})
360363
}
361364

365+
// Test_PgSQL_Returning_CatchSQL verifies CatchSQL captures INSERT ... RETURNING.
366+
// Regression for a bug where pgsql/gaussdb DoExec bypassed Core.DoQuery and
367+
// silently dropped the SQL from CatchSQLManager on InsertAndGetId.
368+
func Test_PgSQL_Returning_CatchSQL(t *testing.T) {
369+
table := fmt.Sprintf(`%s_%d`, TablePrefix+"returning_catchsql", gtime.TimestampNano())
370+
if _, err := db.Exec(ctx, fmt.Sprintf(`
371+
CREATE TABLE %s (
372+
id bigserial PRIMARY KEY,
373+
name varchar(100)
374+
);`, table)); err != nil {
375+
gtest.Fatal(err)
376+
}
377+
defer dropTable(table)
378+
379+
gtest.C(t, func(t *gtest.T) {
380+
sqlArray, err := gdb.CatchSQL(ctx, func(ctx context.Context) error {
381+
_, e := db.Ctx(ctx).Model(table).Data(g.Map{"name": "alice"}).InsertAndGetId()
382+
return e
383+
})
384+
t.AssertNil(err)
385+
t.AssertGT(len(sqlArray), 0)
386+
// The captured SQL must contain the RETURNING clause.
387+
t.Assert(gstr.Contains(sqlArray[len(sqlArray)-1], `RETURNING "id"`), true)
388+
// Insert must have executed (CatchSQL uses DoCommit=true).
389+
count, err := db.Model(table).Count()
390+
t.AssertNil(err)
391+
t.Assert(count, 1)
392+
})
393+
}
394+
395+
// Test_PgSQL_Returning_ToSQL verifies ToSQL captures without executing.
396+
// Regression for a bug where pgsql/gaussdb DoExec bypassed Core.DoQuery and
397+
// executed the INSERT even when ToSQL (DoCommit=false) was active.
398+
func Test_PgSQL_Returning_ToSQL(t *testing.T) {
399+
table := fmt.Sprintf(`%s_%d`, TablePrefix+"returning_tosql", gtime.TimestampNano())
400+
if _, err := db.Exec(ctx, fmt.Sprintf(`
401+
CREATE TABLE %s (
402+
id bigserial PRIMARY KEY,
403+
name varchar(100)
404+
);`, table)); err != nil {
405+
gtest.Fatal(err)
406+
}
407+
defer dropTable(table)
408+
409+
gtest.C(t, func(t *gtest.T) {
410+
before, err := db.Model(table).Count()
411+
t.AssertNil(err)
412+
413+
capturedSql, err := gdb.ToSQL(ctx, func(ctx context.Context) error {
414+
_, e := db.Ctx(ctx).Model(table).Data(g.Map{"name": "bob"}).InsertAndGetId()
415+
return e
416+
})
417+
t.AssertNil(err)
418+
t.AssertNE(capturedSql, "")
419+
// Row count must be unchanged — ToSQL must NOT execute.
420+
after, err := db.Model(table).Count()
421+
t.AssertNil(err)
422+
t.Assert(after, before)
423+
})
424+
}
425+
362426
// =============================================================================
363427
// Layer 3: CTE (Common Table Expression) tests
364428
// =============================================================================

‎contrib/drivers/pgsql/pgsql_z_unit_issue_test.go‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1317,14 +1317,10 @@ func Test_Issue3204(t *testing.T) {
13171317
})
13181318
t.AssertNil(err)
13191319
t.Assert(insertId, 20)
1320-
// CatchSQL may return empty on PgSQL when InsertAndGetId uses RETURNING.
1321-
// The functional assertion (insertId=20) is the meaningful check.
1322-
if len(sqlArray) > 0 {
1323-
t.Assert(
1324-
gstr.Contains(sqlArray[len(sqlArray)-1], `("id","passport") VALUES(20,'passport_20')`),
1325-
true,
1326-
)
1327-
}
1320+
t.Assert(
1321+
gstr.Contains(sqlArray[len(sqlArray)-1], `("id","passport") VALUES(20,'passport_20')`),
1322+
true,
1323+
)
13281324
})
13291325
// update data
13301326
gtest.C(t, func(t *gtest.T) {

0 commit comments

Comments
 (0)