Skip to content

Commit 63f8476

Browse files
committed
opt: add a function to let DrainRecordSet also close the RecordSet
1 parent e7b038b commit 63f8476

File tree

3 files changed

+26
-60
lines changed

3 files changed

+26
-60
lines changed

pkg/executor/simple.go

Lines changed: 10 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -962,12 +962,7 @@ func readPasswordLockingInfo(ctx context.Context, sqlExecutor sqlexec.SQLExecuto
962962
if err != nil {
963963
return nil, err
964964
}
965-
defer func() {
966-
if closeErr := recordSet.Close(); closeErr != nil {
967-
err = closeErr
968-
}
969-
}()
970-
rows, err := sqlexec.DrainRecordSet(ctx, recordSet, 3)
965+
rows, err := sqlexec.DrainRecordSetAndClose(ctx, recordSet, 3)
971966
if err != nil {
972967
return nil, err
973968
}
@@ -1320,12 +1315,7 @@ func isRole(ctx context.Context, sqlExecutor sqlexec.SQLExecutor, name, host str
13201315
if err != nil {
13211316
return false, err
13221317
}
1323-
defer func() {
1324-
if closeErr := recordSet.Close(); closeErr != nil {
1325-
err = closeErr
1326-
}
1327-
}()
1328-
rows, err := sqlexec.DrainRecordSet(ctx, recordSet, 1)
1318+
rows, err := sqlexec.DrainRecordSetAndClose(ctx, recordSet, 1)
13291319
if err != nil {
13301320
return false, err
13311321
}
@@ -1342,12 +1332,7 @@ func getUserPasswordLimit(ctx context.Context, sqlExecutor sqlexec.SQLExecutor,
13421332
if err != nil {
13431333
return nil, err
13441334
}
1345-
defer func() {
1346-
if closeErr := recordSet.Close(); closeErr != nil {
1347-
err = closeErr
1348-
}
1349-
}()
1350-
rows, err := sqlexec.DrainRecordSet(ctx, recordSet, 3)
1335+
rows, err := sqlexec.DrainRecordSetAndClose(ctx, recordSet, 3)
13511336
if err != nil {
13521337
return nil, err
13531338
}
@@ -1464,12 +1449,7 @@ func getUserPasswordNum(ctx context.Context, sqlExecutor sqlexec.SQLExecutor, us
14641449
if err != nil {
14651450
return 0, err
14661451
}
1467-
defer func() {
1468-
if closeErr := recordSet.Close(); closeErr != nil {
1469-
err = closeErr
1470-
}
1471-
}()
1472-
rows, err := sqlexec.DrainRecordSet(ctx, recordSet, 3)
1452+
rows, err := sqlexec.DrainRecordSetAndClose(ctx, recordSet, 3)
14731453
if err != nil {
14741454
return 0, err
14751455
}
@@ -1490,12 +1470,7 @@ func fullRecordCheck(ctx context.Context, sqlExecutor sqlexec.SQLExecutor, userD
14901470
if err != nil {
14911471
return false, err
14921472
}
1493-
defer func() {
1494-
if closeErr := recordSet.Close(); closeErr != nil {
1495-
err = closeErr
1496-
}
1497-
}()
1498-
rows, err := sqlexec.DrainRecordSet(ctx, recordSet, 3)
1473+
rows, err := sqlexec.DrainRecordSetAndClose(ctx, recordSet, 3)
14991474
if err != nil {
15001475
return false, err
15011476
}
@@ -1510,12 +1485,7 @@ func fullRecordCheck(ctx context.Context, sqlExecutor sqlexec.SQLExecutor, userD
15101485
if err != nil {
15111486
return false, err
15121487
}
1513-
defer func() {
1514-
if closeErr := recordSet.Close(); closeErr != nil {
1515-
err = closeErr
1516-
}
1517-
}()
1518-
rows, err := sqlexec.DrainRecordSet(ctx, recordSet, vardef.DefMaxChunkSize)
1488+
rows, err := sqlexec.DrainRecordSetAndClose(ctx, recordSet, vardef.DefMaxChunkSize)
15191489
if err != nil {
15201490
return false, err
15211491
}
@@ -1538,12 +1508,7 @@ func checkPasswordHistoryRule(ctx context.Context, sqlExecutor sqlexec.SQLExecut
15381508
if err != nil {
15391509
return false, err
15401510
}
1541-
defer func() {
1542-
if closeErr := recordSet.Close(); closeErr != nil {
1543-
err = closeErr
1544-
}
1545-
}()
1546-
rows, err := sqlexec.DrainRecordSet(ctx, recordSet, 3)
1511+
rows, err := sqlexec.DrainRecordSetAndClose(ctx, recordSet, 3)
15471512
if err != nil {
15481513
return false, err
15491514
}
@@ -1560,12 +1525,7 @@ func checkPasswordHistoryRule(ctx context.Context, sqlExecutor sqlexec.SQLExecut
15601525
if err != nil {
15611526
return false, err
15621527
}
1563-
defer func() {
1564-
if closeErr := recordSet.Close(); closeErr != nil {
1565-
err = closeErr
1566-
}
1567-
}()
1568-
rows, err := sqlexec.DrainRecordSet(ctx, recordSet, vardef.DefMaxChunkSize)
1528+
rows, err := sqlexec.DrainRecordSetAndClose(ctx, recordSet, vardef.DefMaxChunkSize)
15691529
if err != nil {
15701530
return false, err
15711531
}
@@ -1587,12 +1547,7 @@ func checkPasswordTimeRule(ctx context.Context, sqlExecutor sqlexec.SQLExecutor,
15871547
if err != nil {
15881548
return false, err
15891549
}
1590-
defer func() {
1591-
if closeErr := recordSet.Close(); closeErr != nil {
1592-
err = closeErr
1593-
}
1594-
}()
1595-
rows, err := sqlexec.DrainRecordSet(ctx, recordSet, 3)
1550+
rows, err := sqlexec.DrainRecordSetAndClose(ctx, recordSet, 3)
15961551
if err != nil {
15971552
return false, err
15981553
}
@@ -1606,12 +1561,7 @@ func checkPasswordTimeRule(ctx context.Context, sqlExecutor sqlexec.SQLExecutor,
16061561
if err != nil {
16071562
return false, err
16081563
}
1609-
defer func() {
1610-
if closeErr := recordSet.Close(); closeErr != nil {
1611-
err = closeErr
1612-
}
1613-
}()
1614-
rows, err := sqlexec.DrainRecordSet(ctx, recordSet, vardef.DefMaxChunkSize)
1564+
rows, err := sqlexec.DrainRecordSetAndClose(ctx, recordSet, vardef.DefMaxChunkSize)
16151565
if err != nil {
16161566
return false, err
16171567
}

pkg/util/sqlexec/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ go_library(
1717
"//pkg/sessionctx/variable",
1818
"//pkg/types",
1919
"//pkg/util/chunk",
20+
"//pkg/util/logutil",
21+
"@org_uber_go_zap//:zap",
2022
],
2123
)
2224

pkg/util/sqlexec/restricted_sql_executor.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ import (
2424
"github.com/pingcap/tidb/pkg/sessionctx/sysproctrack"
2525
"github.com/pingcap/tidb/pkg/sessionctx/variable"
2626
"github.com/pingcap/tidb/pkg/util/chunk"
27+
"github.com/pingcap/tidb/pkg/util/logutil"
28+
"go.uber.org/zap"
2729
)
2830

2931
// RestrictedSQLExecutor is an interface provides executing restricted sql statement.
@@ -250,6 +252,18 @@ func DrainRecordSet(ctx context.Context, rs RecordSet, maxChunkSize int) ([]chun
250252
}
251253
}
252254

255+
// DrainRecordSetAndClose fetches the rows in the RecordSet and closes it.
256+
func DrainRecordSetAndClose(ctx context.Context, rs RecordSet, maxChunkSize int) ([]chunk.Row, error) {
257+
defer func() {
258+
if closeErr := rs.Close(); closeErr != nil {
259+
// Log the close error but don't override the main error
260+
logutil.BgLogger().Error("failed to close recordSet in DrainRecordSetAndClose", zap.Error(closeErr))
261+
}
262+
}()
263+
264+
return DrainRecordSet(ctx, rs, maxChunkSize)
265+
}
266+
253267
// ExecSQL executes the sql and returns the result.
254268
// TODO: consider retry.
255269
func ExecSQL(ctx context.Context, exec SQLExecutor, sql string, args ...any) ([]chunk.Row, error) {

0 commit comments

Comments
 (0)