Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ddl: migrate part of ddl package code from Execute/ExecRestricted to safe API (1) #22670

Merged
merged 17 commits into from
Feb 5, 2021
Merged

ddl: migrate part of ddl package code from Execute/ExecRestricted to safe API (1) #22670

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
017c69c
migrate part of ddl package code from Execute/ExecRestricted to safe API
AilinKid Feb 2, 2021
d227711
Merge branch 'master' into fix_ddl_package
AilinKid Feb 2, 2021
d768063
address comment
AilinKid Feb 3, 2021
3d25f86
Merge branch 'master' into fix_ddl_package
AilinKid Feb 3, 2021
0f19818
Merge branch 'master' into fix_ddl_package
AilinKid Feb 3, 2021
ba73205
make fmt
AilinKid Feb 3, 2021
29a6f68
.
AilinKid Feb 3, 2021
fad310d
restore the old api in the test
AilinKid Feb 3, 2021
c1f8106
Merge branch 'master' into fix_ddl_package
AilinKid Feb 3, 2021
7a8f1ff
address comment
AilinKid Feb 3, 2021
a863e4f
Merge branch 'master' into fix_ddl_package
AilinKid Feb 3, 2021
f3b9380
.
AilinKid Feb 4, 2021
cef2347
.
AilinKid Feb 4, 2021
3a680c4
Merge branch 'master' into fix_ddl_package
AilinKid Feb 4, 2021
1d5c0c1
Merge branch 'master' into fix_ddl_package
AilinKid Feb 4, 2021
f8a6282
Merge branch 'master' into fix_ddl_package
AilinKid Feb 4, 2021
65dc56f
Merge branch 'master' into fix_ddl_package
AilinKid Feb 5, 2021
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
8 changes: 6 additions & 2 deletions ddl/column.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1542,8 +1542,12 @@ func checkForNullValue(ctx sessionctx.Context, isDataTruncated bool, schema, tab
paramsList = append(paramsList, col.Name.L)
}
}
sql := "select 1 from %n.%n where " + colsStr + " limit 1;"
stmt, err := ctx.(sqlexec.RestrictedSQLExecutor).ParseWithParams(context.Background(), sql, paramsList...)
var buf strings.Builder
_, err := fmt.Fprintf(&buf, "select 1 from %%n.%%n where %s limit 1", colsStr)
AilinKid marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return errors.Trace(err)
}
stmt, err := ctx.(sqlexec.RestrictedSQLExecutor).ParseWithParams(context.Background(), buf.String(), paramsList...)
if err != nil {
return errors.Trace(err)
}
Expand Down
38 changes: 23 additions & 15 deletions ddl/util/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ package util
import (
"context"
"encoding/hex"
"fmt"
"strings"

"github.com/pingcap/errors"
"github.com/pingcap/parser/terror"
Expand All @@ -27,15 +29,14 @@ import (
)

const (
deleteRangesTable = `gc_delete_range`
doneDeleteRangesTable = `gc_delete_range_done`
loadDeleteRangeSQL = `SELECT HIGH_PRIORITY job_id, element_id, start_key, end_key FROM mysql.%n WHERE ts < %?`
recordDoneDeletedRangeSQL = `INSERT IGNORE INTO mysql.gc_delete_range_done SELECT * FROM mysql.gc_delete_range WHERE job_id = %? AND element_id = %?`
completeDeleteRangeSQL = `DELETE FROM mysql.gc_delete_range WHERE job_id = %? AND element_id = %?`
completeDeleteMultiRangesSQLPrefix = `DELETE FROM mysql.gc_delete_range WHERE job_id = %? AND element_id in (`
completeDeleteMultiRangesSQLSuffix = `)`
updateDeleteRangeSQL = `UPDATE mysql.gc_delete_range SET start_key = %? WHERE job_id = %? AND element_id = %? AND start_key = %?`
deleteDoneRecordSQL = `DELETE FROM mysql.gc_delete_range_done WHERE job_id = %? AND element_id = %?`
deleteRangesTable = `gc_delete_range`
doneDeleteRangesTable = `gc_delete_range_done`
loadDeleteRangeSQL = `SELECT HIGH_PRIORITY job_id, element_id, start_key, end_key FROM mysql.%n WHERE ts < %?`
recordDoneDeletedRangeSQL = `INSERT IGNORE INTO mysql.gc_delete_range_done SELECT * FROM mysql.gc_delete_range WHERE job_id = %? AND element_id = %?`
completeDeleteRangeSQL = `DELETE FROM mysql.gc_delete_range WHERE job_id = %? AND element_id = %?`
completeDeleteMultiRangesSQL = `DELETE FROM mysql.gc_delete_range WHERE job_id = %%? AND element_id in (%s)`
AilinKid marked this conversation as resolved.
Show resolved Hide resolved
updateDeleteRangeSQL = `UPDATE mysql.gc_delete_range SET start_key = %? WHERE job_id = %? AND element_id = %? AND start_key = %?`
deleteDoneRecordSQL = `DELETE FROM mysql.gc_delete_range_done WHERE job_id = %? AND element_id = %?`
)

// DelRangeTask is for run delete-range command in gc_worker.
Expand Down Expand Up @@ -128,8 +129,12 @@ func RemoveMultiFromGCDeleteRange(ctx sessionctx.Context, jobID int64, elementID
idList += "%?"
paramIDs = append(paramIDs, elementID)
}
completeDeleteMultiRangesSQL := completeDeleteMultiRangesSQLPrefix + idList + completeDeleteMultiRangesSQLSuffix
_, err := ctx.(sqlexec.SQLExecutor).ExecuteInternal(context.TODO(), completeDeleteMultiRangesSQL, paramIDs...)
var buf strings.Builder
_, err := fmt.Fprintf(&buf, completeDeleteMultiRangesSQL, idList)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just let you know, we have fmt.Fprintf in our errcheck_ignore lists. And I agreed with that. fmt.Fprintf can not throw error on valid input, unless OOM. So you do not need to check that error.

BTW, You could use Fprintf to concat idList & nameList too.

if err != nil {
return errors.Trace(err)
}
_, err = ctx.(sqlexec.SQLExecutor).ExecuteInternal(context.TODO(), buf.String(), paramIDs...)
return errors.Trace(err)
}

Expand Down Expand Up @@ -160,8 +165,7 @@ func LoadDDLVars(ctx sessionctx.Context) error {
}

const (
loadGlobalVarsSQLPrefix = "select HIGH_PRIORITY variable_name, variable_value from mysql.global_variables where variable_name in ("
loadGlobalVarsSQLSuffix = ")"
loadGlobalVarsSQL = "select HIGH_PRIORITY variable_name, variable_value from mysql.global_variables where variable_name in (%s)"
)

// LoadGlobalVars loads global variable from mysql.global_variables.
Expand All @@ -176,8 +180,12 @@ func LoadGlobalVars(ctx sessionctx.Context, varNames []string) error {
nameList += "%?"
paramNames = append(paramNames, name)
}
loadGlobalVarsSQL := loadGlobalVarsSQLPrefix + nameList + loadGlobalVarsSQLSuffix
stmt, err := sctx.ParseWithParams(context.Background(), loadGlobalVarsSQL, paramNames...)
var buf strings.Builder
_, err := fmt.Fprintf(&buf, loadGlobalVarsSQL, nameList)
if err != nil {
return errors.Trace(err)
}
stmt, err := sctx.ParseWithParams(context.Background(), buf.String(), paramNames...)
if err != nil {
return errors.Trace(err)
}
Expand Down