Skip to content

Commit 3c44cad

Browse files
ZhouXing19ZhouXing19
committed
filetable: use CollectionFactory.TxnWithExecutor() for DDL statements
fixes #76764 Release justification: Low risk, high benefit changes to existing functionality Release note: none
1 parent 22e7027 commit 3c44cad

2 files changed

Lines changed: 69 additions & 23 deletions

File tree

‎pkg/ccl/backupccl/testdata/backup-restore/file_table_read_write‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
subtest backup_file_table
2+
3+
new-server name=s1
4+
----
5+
6+
exec-sql
7+
CREATE DATABASE to_backup;
8+
----
9+
10+
exec-sql
11+
CREATE DATABASE backups;
12+
----
13+
14+
exec-sql
15+
BACKUP DATABASE to_backup INTO 'userfile://backups.public.userfiles_$user/data';
16+
----
17+
18+
query-sql
19+
SELECT * FROM backups.crdb_internal.invalid_objects;
20+
----
21+
22+
exec-sql
23+
USE backups;
24+
----
25+
26+
query-sql
27+
SELECT * FROM pg_catalog.pg_tables where schemaname='public';
28+
----
29+
public userfiles_$user_upload_files root <nil> true false false false
30+
public userfiles_$user_upload_payload root <nil> true false false false
31+
32+
query-sql
33+
SELECT conname FROM pg_catalog.pg_constraint con
34+
INNER JOIN pg_catalog.pg_class rel ON rel.oid = con.conrelid
35+
INNER JOIN pg_catalog.pg_namespace nsp
36+
ON nsp.oid = connamespace
37+
WHERE rel.relname='userfiles_$user_upload_payload'
38+
ORDER BY conname;
39+
----
40+
file_id_fk
41+
userfiles_$user_upload_payload_pkey
42+
43+
subtest end

‎pkg/cloud/userfile/filetable/file_table_read_writer.go‎

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -245,25 +245,27 @@ func NewFileToTableSystem(
245245
if err != nil {
246246
return nil, err
247247
}
248-
if err := e.db.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error {
248+
if err := e.cf.TxnWithExecutor(ctx, e.db, nil /* SessionData */, func(
249+
ctx context.Context, txn *kv.Txn, descriptors *descs.Collection, ie sqlutil.InternalExecutor,
250+
) error {
249251
// TODO(adityamaru): Handle scenario where the user has already created
250252
// tables with the same names not via the FileToTableSystem
251253
// object. Not sure if we want to error out or work around it.
252-
tablesExist, err := f.checkIfFileAndPayloadTableExist(ctx, txn, e.ie)
254+
tablesExist, err := f.checkIfFileAndPayloadTableExist(ctx, txn, ie)
253255
if err != nil {
254256
return err
255257
}
256258

257259
if !tablesExist {
258-
if err := f.createFileAndPayloadTables(ctx, txn, e.ie); err != nil {
260+
if err := f.createFileAndPayloadTables(ctx, txn, ie); err != nil {
259261
return err
260262
}
261263

262-
if err := f.grantCurrentUserTablePrivileges(ctx, txn, e.ie); err != nil {
264+
if err := f.grantCurrentUserTablePrivileges(ctx, txn, ie); err != nil {
263265
return err
264266
}
265267

266-
if err := f.revokeOtherUserTablePrivileges(ctx, txn, e.ie); err != nil {
268+
if err := f.revokeOtherUserTablePrivileges(ctx, txn, ie); err != nil {
267269
return err
268270
}
269271
}
@@ -364,26 +366,27 @@ func DestroyUserFileSystem(ctx context.Context, f *FileToTableSystem) error {
364366
return err
365367
}
366368

367-
if err := e.db.Txn(ctx,
368-
func(ctx context.Context, txn *kv.Txn) error {
369-
dropPayloadTableQuery := fmt.Sprintf(`DROP TABLE %s`, f.GetFQPayloadTableName())
370-
_, err := e.ie.ExecEx(ctx, "drop-payload-table", txn,
371-
sessiondata.InternalExecutorOverride{User: f.username},
372-
dropPayloadTableQuery)
373-
if err != nil {
374-
return errors.Wrap(err, "failed to drop payload table")
375-
}
369+
if err := e.cf.TxnWithExecutor(ctx, e.db, nil, func(
370+
ctx context.Context, txn *kv.Txn, descriptors *descs.Collection, ie sqlutil.InternalExecutor,
371+
) error {
372+
dropPayloadTableQuery := fmt.Sprintf(`DROP TABLE %s`, f.GetFQPayloadTableName())
373+
_, err := ie.ExecEx(ctx, "drop-payload-table", txn,
374+
sessiondata.InternalExecutorOverride{User: f.username},
375+
dropPayloadTableQuery)
376+
if err != nil {
377+
return errors.Wrap(err, "failed to drop payload table")
378+
}
376379

377-
dropFileTableQuery := fmt.Sprintf(`DROP TABLE %s CASCADE`, f.GetFQFileTableName())
378-
_, err = e.ie.ExecEx(ctx, "drop-file-table", txn,
379-
sessiondata.InternalExecutorOverride{User: f.username},
380-
dropFileTableQuery)
381-
if err != nil {
382-
return errors.Wrap(err, "failed to drop file table")
383-
}
380+
dropFileTableQuery := fmt.Sprintf(`DROP TABLE %s CASCADE`, f.GetFQFileTableName())
381+
_, err = ie.ExecEx(ctx, "drop-file-table", txn,
382+
sessiondata.InternalExecutorOverride{User: f.username},
383+
dropFileTableQuery)
384+
if err != nil {
385+
return errors.Wrap(err, "failed to drop file table")
386+
}
384387

385-
return nil
386-
}); err != nil {
388+
return nil
389+
}); err != nil {
387390
return err
388391
}
389392

0 commit comments

Comments
 (0)