Skip to content

Commit ae6e334

Browse files
committed
refactor(drivers): Unify table name quoting
- Quote table names using double quotes ("). - For MySQL, add `ANSI_QUOTES` to the `sql_mode` parameter in the DSN. Signed-off-by: happy game <[email protected]>
1 parent cf0dbf6 commit ae6e334

File tree

4 files changed

+86
-83
lines changed

4 files changed

+86
-83
lines changed

pkg/drivers/generic/generic.go

Lines changed: 28 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,11 @@ const (
3030
var _ server.Dialect = (*Generic)(nil)
3131

3232
var (
33-
columns = "kv.id AS theid, kv.name AS thename, kv.created, kv.deleted, kv.create_revision, kv.prev_revision, kv.lease, kv.value, kv.old_value"
34-
revSQL string
35-
compactRevSQL string
36-
listSQL string
37-
tableName string
38-
quotedTableName string
33+
columns = `kv.id AS theid, kv.name AS thename, kv.created, kv.deleted, kv.create_revision, kv.prev_revision, kv.lease, kv.value, kv.old_value`
34+
revSQL string
35+
compactRevSQL string
36+
listSQL string
37+
tableName string
3938
)
4039

4140
type ErrRetry func(error) bool
@@ -96,8 +95,8 @@ func q(sql, param string, numbered bool) string {
9695
func (d *Generic) Migrate(ctx context.Context) {
9796
var (
9897
count = 0
99-
countKV = d.queryRow(ctx, "SELECT COUNT(*) FROM key_value")
100-
countKine = d.queryRow(ctx, "SELECT COUNT(*) FROM "+quotedTableName)
98+
countKV = d.queryRow(ctx, `SELECT COUNT(*) FROM key_value`)
99+
countKine = d.queryRow(ctx, `SELECT COUNT(*) FROM "`+tableName+`"`)
101100
)
102101

103102
if err := countKV.Scan(&count); err != nil || count == 0 {
@@ -110,7 +109,7 @@ func (d *Generic) Migrate(ctx context.Context) {
110109

111110
logrus.Infof("Migrating content from old table")
112111
_, err := d.execute(ctx,
113-
`INSERT INTO `+quotedTableName+`(deleted, create_revision, prev_revision, name, value, created, lease)
112+
`INSERT INTO "`+tableName+`"(deleted, create_revision, prev_revision, name, value, created, lease)
114113
SELECT 0, 0, 0, kv.name, kv.value, 1, CASE WHEN kv.ttl > 0 THEN 15 ELSE 0 END
115114
FROM key_value kv
116115
WHERE kv.id IN (SELECT MAX(kvd.id) FROM key_value kvd GROUP BY kvd.name)`)
@@ -151,21 +150,21 @@ func validateTableName(customTableName string) error {
151150
func buildSQLStatements() (rev, compactRev, list string) {
152151
rev = fmt.Sprintf(`
153152
SELECT MAX(rkv.id) AS id
154-
FROM %s AS rkv`, quotedTableName)
153+
FROM "%s" AS rkv`, tableName)
155154

156155
compactRev = fmt.Sprintf(`
157156
SELECT MAX(crkv.prev_revision) AS prev_revision
158-
FROM %s AS crkv
159-
WHERE crkv.name = 'compact_rev_key'`, quotedTableName)
157+
FROM "%s" AS crkv
158+
WHERE crkv.name = 'compact_rev_key'`, tableName)
160159

161160
list = fmt.Sprintf(`
162161
SELECT *
163162
FROM (
164163
SELECT (%s), (%s), %s
165-
FROM %s AS kv
164+
FROM "%s" AS kv
166165
JOIN (
167166
SELECT MAX(mkv.id) AS id
168-
FROM %s AS mkv
167+
FROM "%s" AS mkv
169168
WHERE
170169
mkv.name LIKE ?
171170
%%s
@@ -176,7 +175,7 @@ func buildSQLStatements() (rev, compactRev, list string) {
176175
?
177176
) AS lkv
178177
ORDER BY lkv.thename ASC
179-
`, rev, compactRev, columns, quotedTableName, quotedTableName)
178+
`, rev, compactRev, columns, tableName, tableName)
180179

181180
return rev, compactRev, list
182181
}
@@ -208,16 +207,6 @@ func Open(ctx context.Context, driverName, dataSourceName string, connPoolConfig
208207
}
209208

210209
tableName = customTableName
211-
212-
// In case of MySQL, we need to quote the table name using ` `
213-
// In case of SQLite and Postgres, we need to quote the table name using " "
214-
switch driverName {
215-
case "mysql":
216-
quotedTableName = "`" + tableName + "`"
217-
default:
218-
quotedTableName = `"` + tableName + `"`
219-
}
220-
221210
revSQL, compactRevSQL, listSQL = buildSQLStatements()
222211

223212
for i := 0; i < 300; i++ {
@@ -246,8 +235,8 @@ func Open(ctx context.Context, driverName, dataSourceName string, connPoolConfig
246235
GetRevisionSQL: q(fmt.Sprintf(`
247236
SELECT
248237
0, 0, %s
249-
FROM %s AS kv
250-
WHERE kv.id = ?`, columns, quotedTableName), paramCharacter, numbered),
238+
FROM "%s" AS kv
239+
WHERE kv.id = ?`, columns, tableName), paramCharacter, numbered),
251240

252241
GetCurrentSQL: q(fmt.Sprintf(listSQL, "AND mkv.name > ?"), paramCharacter, numbered),
253242
ListRevisionStartSQL: q(fmt.Sprintf(listSQL, "AND mkv.id <= ?"), paramCharacter, numbered),
@@ -267,29 +256,29 @@ func Open(ctx context.Context, driverName, dataSourceName string, connPoolConfig
267256

268257
AfterSQL: q(fmt.Sprintf(`
269258
SELECT (%s), (%s), %s
270-
FROM %s AS kv
259+
FROM "%s" AS kv
271260
WHERE
272261
kv.name LIKE ? AND
273262
kv.id > ?
274-
ORDER BY kv.id ASC`, revSQL, compactRevSQL, columns, quotedTableName), paramCharacter, numbered),
263+
ORDER BY kv.id ASC`, revSQL, compactRevSQL, columns, tableName), paramCharacter, numbered),
275264

276265
DeleteSQL: q(fmt.Sprintf(`
277-
DELETE FROM %s AS kv
278-
WHERE kv.id = ?`, quotedTableName), paramCharacter, numbered),
266+
DELETE FROM "%s" AS kv
267+
WHERE kv.id = ?`, tableName), paramCharacter, numbered),
279268

280269
UpdateCompactSQL: q(fmt.Sprintf(`
281-
UPDATE %s
270+
UPDATE "%s"
282271
SET prev_revision = ?
283-
WHERE name = 'compact_rev_key'`, quotedTableName), paramCharacter, numbered),
272+
WHERE name = 'compact_rev_key'`, tableName), paramCharacter, numbered),
284273

285-
InsertLastInsertIDSQL: q(fmt.Sprintf(`INSERT INTO %s(name, created, deleted, create_revision, prev_revision, lease, value, old_value)
286-
values(?, ?, ?, ?, ?, ?, ?, ?)`, quotedTableName), paramCharacter, numbered),
274+
InsertLastInsertIDSQL: q(fmt.Sprintf(`INSERT INTO "%s"(name, created, deleted, create_revision, prev_revision, lease, value, old_value)
275+
values(?, ?, ?, ?, ?, ?, ?, ?)`, tableName), paramCharacter, numbered),
287276

288-
InsertSQL: q(fmt.Sprintf(`INSERT INTO %s(name, created, deleted, create_revision, prev_revision, lease, value, old_value)
289-
values(?, ?, ?, ?, ?, ?, ?, ?) RETURNING id`, quotedTableName), paramCharacter, numbered),
277+
InsertSQL: q(fmt.Sprintf(`INSERT INTO "%s"(name, created, deleted, create_revision, prev_revision, lease, value, old_value)
278+
values(?, ?, ?, ?, ?, ?, ?, ?) RETURNING id`, tableName), paramCharacter, numbered),
290279

291-
FillSQL: q(fmt.Sprintf(`INSERT INTO %s(id, name, created, deleted, create_revision, prev_revision, lease, value, old_value)
292-
values(?, ?, ?, ?, ?, ?, ?, ?, ?)`, quotedTableName), paramCharacter, numbered),
280+
FillSQL: q(fmt.Sprintf(`INSERT INTO "%s"(id, name, created, deleted, create_revision, prev_revision, lease, value, old_value)
281+
values(?, ?, ?, ?, ?, ?, ?, ?, ?)`, tableName), paramCharacter, numbered),
293282
}, err
294283
}
295284

pkg/drivers/mysql/mysql.go

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"fmt"
88
"os"
99
"strconv"
10+
"strings"
1011

1112
"github.com/go-sql-driver/mysql"
1213
"github.com/sirupsen/logrus"
@@ -27,9 +28,8 @@ const (
2728
var createDB = "CREATE DATABASE IF NOT EXISTS `%s`;"
2829

2930
func getSchema(tableName string) []string {
30-
quotedTableName := "`" + tableName + "`"
3131
return []string{
32-
`CREATE TABLE IF NOT EXISTS ` + quotedTableName + `
32+
`CREATE TABLE IF NOT EXISTS "` + tableName + `"
3333
(
3434
id BIGINT UNSIGNED AUTO_INCREMENT,
3535
name VARCHAR(630) CHARACTER SET ascii,
@@ -42,18 +42,17 @@ func getSchema(tableName string) []string {
4242
old_value MEDIUMBLOB,
4343
PRIMARY KEY (id)
4444
);`,
45-
"CREATE INDEX `" + tableName + "_name_index` ON " + quotedTableName + " (name)",
46-
"CREATE INDEX `" + tableName + "_name_id_index` ON " + quotedTableName + " (name,id)",
47-
"CREATE INDEX `" + tableName + "_id_deleted_index` ON " + quotedTableName + " (id,deleted)",
48-
"CREATE INDEX `" + tableName + "_prev_revision_index` ON " + quotedTableName + " (prev_revision)",
49-
"CREATE UNIQUE INDEX `" + tableName + "_name_prev_revision_uindex` ON " + quotedTableName + " (name, prev_revision)",
45+
`CREATE INDEX "` + tableName + `_name_index" ON "` + tableName + `" (name)`,
46+
`CREATE INDEX "` + tableName + `_name_id_index" ON "` + tableName + `" (name,id)`,
47+
`CREATE INDEX "` + tableName + `_id_deleted_index" ON "` + tableName + `" (id,deleted)`,
48+
`CREATE INDEX "` + tableName + `_prev_revision_index" ON "` + tableName + `" (prev_revision)`,
49+
`CREATE UNIQUE INDEX "` + tableName + `_name_prev_revision_uindex" ON "` + tableName + `" (name, prev_revision)`,
5050
}
5151
}
5252

5353
func getSchemaMigrations(tableName string) []string {
54-
quotedTableName := "`" + tableName + "`"
5554
return []string{
56-
`ALTER TABLE ` + quotedTableName + ` MODIFY COLUMN id BIGINT UNSIGNED AUTO_INCREMENT NOT NULL UNIQUE, MODIFY COLUMN create_revision BIGINT UNSIGNED, MODIFY COLUMN prev_revision BIGINT UNSIGNED`,
55+
`ALTER TABLE "` + tableName + `" MODIFY COLUMN id BIGINT UNSIGNED AUTO_INCREMENT NOT NULL UNIQUE, MODIFY COLUMN create_revision BIGINT UNSIGNED, MODIFY COLUMN prev_revision BIGINT UNSIGNED`,
5756
// Creating an empty migration to ensure that postgresql and mysql migrations match up
5857
// with each other for a give value of KINE_SCHEMA_MIGRATION env var
5958
``,
@@ -83,7 +82,6 @@ func New(ctx context.Context, cfg *drivers.Config) (bool, server.Backend, error)
8382
if tableName == "" {
8483
tableName = "kine"
8584
}
86-
quotedTableName := "`" + tableName + "`"
8785

8886
dialect, err := generic.Open(ctx, "mysql", parsedDSN, cfg.ConnectionPoolConfig, "?", false, cfg.MetricsRegisterer, tableName)
8987
if err != nil {
@@ -96,17 +94,17 @@ func New(ctx context.Context, cfg *drivers.Config) (bool, server.Backend, error)
9694
FROM information_schema.TABLES
9795
WHERE table_schema = DATABASE() AND table_name = '` + tableName + `'`
9896
dialect.CompactSQL = `
99-
DELETE kv FROM ` + quotedTableName + ` AS kv
97+
DELETE kv FROM "` + tableName + `" AS kv
10098
INNER JOIN (
10199
SELECT kp.prev_revision AS id
102-
FROM ` + quotedTableName + ` AS kp
100+
FROM "` + tableName + `" AS kp
103101
WHERE
104102
kp.name != 'compact_rev_key' AND
105103
kp.prev_revision != 0 AND
106104
kp.id <= ?
107105
UNION
108106
SELECT kd.id AS id
109-
FROM ` + quotedTableName + ` AS kd
107+
FROM "` + tableName + `" AS kd
110108
WHERE
111109
kd.deleted != 0 AND
112110
kd.id <= ?
@@ -228,6 +226,27 @@ func prepareDSN(dataSourceName string, tlsConfig *cryptotls.Config) (string, err
228226
if err != nil {
229227
return "", err
230228
}
229+
230+
// ensure that ASNI_QUOTES is set in sql_mode
231+
// this is required for using "" for quoting identifiers
232+
// https://dev.mysql.com/doc/refman/8.0/en/sql-mode.html#sqlmode_ansi_quotes
233+
if config.Params == nil {
234+
config.Params = map[string]string{}
235+
}
236+
237+
if mode, exists := config.Params["sql_mode"]; exists {
238+
// check if ANSI_QUOTES is already set
239+
if !strings.Contains(strings.ToUpper(mode), "ANSI_QUOTES") {
240+
if mode == "" {
241+
config.Params["sql_mode"] = "ANSI_QUOTES"
242+
} else {
243+
config.Params["sql_mode"] = mode + ",ANSI_QUOTES"
244+
}
245+
}
246+
} else {
247+
config.Params["sql_mode"] = "ANSI_QUOTES"
248+
}
249+
231250
// setting up tlsConfig
232251
if tlsConfig != nil {
233252
if err := mysql.RegisterTLSConfig("kine", tlsConfig); err != nil {

pkg/drivers/pgsql/pgsql.go

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,8 @@ const (
3131
var createDB = `CREATE DATABASE "%s";`
3232

3333
func getSchema(tableName string) []string {
34-
quotedTableName := `"` + tableName + `"`
3534
return []string{
36-
`CREATE TABLE IF NOT EXISTS ` + quotedTableName + `
35+
`CREATE TABLE IF NOT EXISTS "` + tableName + `"
3736
(
3837
id BIGSERIAL PRIMARY KEY,
3938
name text COLLATE "C",
@@ -46,22 +45,21 @@ func getSchema(tableName string) []string {
4645
old_value bytea
4746
);`,
4847

49-
`CREATE INDEX IF NOT EXISTS "` + tableName + `_name_index" ON ` + quotedTableName + ` (name)`,
50-
`CREATE INDEX IF NOT EXISTS "` + tableName + `_name_id_index" ON ` + quotedTableName + ` (name,id)`,
51-
`CREATE INDEX IF NOT EXISTS "` + tableName + `_id_deleted_index" ON ` + quotedTableName + ` (id,deleted)`,
52-
`CREATE INDEX IF NOT EXISTS "` + tableName + `_prev_revision_index" ON ` + quotedTableName + ` (prev_revision)`,
53-
`CREATE UNIQUE INDEX IF NOT EXISTS "` + tableName + `_name_prev_revision_uindex" ON ` + quotedTableName + ` (name, prev_revision)`,
54-
`CREATE INDEX IF NOT EXISTS "` + tableName + `_list_query_index" on ` + quotedTableName + `(name, id DESC, deleted)`,
48+
`CREATE INDEX IF NOT EXISTS "` + tableName + `_name_index" ON "` + tableName + `" (name)`,
49+
`CREATE INDEX IF NOT EXISTS "` + tableName + `_name_id_index" ON "` + tableName + `" (name,id)`,
50+
`CREATE INDEX IF NOT EXISTS "` + tableName + `_id_deleted_index" ON "` + tableName + `" (id,deleted)`,
51+
`CREATE INDEX IF NOT EXISTS "` + tableName + `_prev_revision_index" ON "` + tableName + `" (prev_revision)`,
52+
`CREATE UNIQUE INDEX IF NOT EXISTS "` + tableName + `_name_prev_revision_uindex" ON "` + tableName + `" (name, prev_revision)`,
53+
`CREATE INDEX IF NOT EXISTS "` + tableName + `_list_query_index" on "` + tableName + `"(name, id DESC, deleted)`,
5554
}
5655
}
5756

5857
func getSchemaMigrations(tableName string) []string {
59-
quotedTableName := `"` + tableName + `"`
6058
return []string{
61-
`ALTER TABLE ` + quotedTableName + ` ALTER COLUMN id SET DATA TYPE BIGINT, ALTER COLUMN create_revision SET DATA TYPE BIGINT, ALTER COLUMN prev_revision SET DATA TYPE BIGINT; ALTER SEQUENCE "` + tableName + `_id_seq" AS BIGINT`,
59+
`ALTER TABLE "` + tableName + `" ALTER COLUMN id SET DATA TYPE BIGINT, ALTER COLUMN create_revision SET DATA TYPE BIGINT, ALTER COLUMN prev_revision SET DATA TYPE BIGINT; ALTER SEQUENCE "` + tableName + `_id_seq" AS BIGINT`,
6260
// It is important to set the collation to "C" to ensure that LIKE and COMPARISON
6361
// queries use the index.
64-
`ALTER TABLE ` + quotedTableName + ` ALTER COLUMN name SET DATA TYPE TEXT COLLATE "C" USING name::TEXT COLLATE "C"`,
62+
`ALTER TABLE "` + tableName + `" ALTER COLUMN name SET DATA TYPE TEXT COLLATE "C" USING name::TEXT COLLATE "C"`,
6563
}
6664
}
6765

@@ -79,22 +77,21 @@ func New(ctx context.Context, cfg *drivers.Config) (bool, server.Backend, error)
7977
if tableName == "" {
8078
tableName = "kine"
8179
}
82-
quotedTableName := `"` + tableName + `"`
8380

8481
dialect, err := generic.Open(ctx, "pgx", parsedDSN, cfg.ConnectionPoolConfig, "$", true, cfg.MetricsRegisterer, tableName)
8582
if err != nil {
8683
return false, nil, err
8784
}
8885
listSQL := `
8986
SELECT
90-
(SELECT MAX(rkv.id) AS id FROM ` + quotedTableName + ` AS rkv),
91-
(SELECT MAX(crkv.prev_revision) AS prev_revision FROM ` + quotedTableName + ` AS crkv WHERE crkv.name = 'compact_rev_key'),
87+
(SELECT MAX(rkv.id) AS id FROM "` + tableName + `" AS rkv),
88+
(SELECT MAX(crkv.prev_revision) AS prev_revision FROM "` + tableName + `" AS crkv WHERE crkv.name = 'compact_rev_key'),
9289
maxkv.*
9390
FROM (
9491
SELECT DISTINCT ON (name)
9592
kv.id AS theid, kv.name, kv.created, kv.deleted, kv.create_revision, kv.prev_revision, kv.lease, kv.value, kv.old_value
9693
FROM
97-
` + quotedTableName + ` AS kv
94+
"` + tableName + `" AS kv
9895
WHERE
9996
kv.name LIKE ?
10097
%s
@@ -107,12 +104,12 @@ func New(ctx context.Context, cfg *drivers.Config) (bool, server.Backend, error)
107104

108105
countSQL := `
109106
SELECT
110-
(SELECT MAX(rkv.id) AS id FROM ` + quotedTableName + ` AS rkv),
107+
(SELECT MAX(rkv.id) AS id FROM "` + tableName + `" AS rkv),
111108
COUNT(c.theid)
112109
FROM (
113110
SELECT DISTINCT ON (name)
114111
kv.id AS theid, kv.deleted
115-
FROM ` + quotedTableName + ` AS kv
112+
FROM "` + tableName + `" AS kv
116113
WHERE
117114
kv.name LIKE ?
118115
%s
@@ -122,17 +119,17 @@ func New(ctx context.Context, cfg *drivers.Config) (bool, server.Backend, error)
122119
`
123120
dialect.GetSizeSQL = `SELECT pg_total_relation_size('` + tableName + `')`
124121
dialect.CompactSQL = `
125-
DELETE FROM ` + quotedTableName + ` AS kv
122+
DELETE FROM "` + tableName + `" AS kv
126123
USING (
127124
SELECT kp.prev_revision AS id
128-
FROM ` + quotedTableName + ` AS kp
125+
FROM "` + tableName + `" AS kp
129126
WHERE
130127
kp.name != 'compact_rev_key' AND
131128
kp.prev_revision != 0 AND
132129
kp.id <= $1
133130
UNION
134131
SELECT kd.id AS id
135-
FROM ` + quotedTableName + ` AS kd
132+
FROM "` + tableName + `" AS kd
136133
WHERE
137134
kd.deleted != 0 AND
138135
kd.id <= $2

0 commit comments

Comments
 (0)