Skip to content

Commit 50a0408

Browse files
authored
fix(table editor): prevent unnecessary update requests (supabase#40192)
Continues on from a0afa9e, reducing the number of unnecessary requests made when table changes are saved. Currently, when you open the "Edit table" side panel, make zero changes, and click "Save", an unnecessary request runs to: - Alter the table's comment to its existing comment - Alter the table's schema to its existing schema - Alter the table's RLS enable status to its existing RLS enable status This PR changes the payload so we only alter what has actually changed. It also adds some refactoring to define clearer types and add type safety for the three possible kinds of table save actions (create new, duplicate, update existing). After this, there are still a few unnecessary column update actions happening on array columns; those will be fixed in a final PR.
1 parent 1e0f7b3 commit 50a0408

File tree

8 files changed

+207
-131
lines changed

8 files changed

+207
-131
lines changed

apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/SidePanelEditor.tsx

Lines changed: 100 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,61 @@ import { SpreadsheetImport } from './SpreadsheetImport/SpreadsheetImport'
5252
import { TableEditor } from './TableEditor/TableEditor'
5353
import type { ImportContent } from './TableEditor/TableEditor.types'
5454

55+
export type SaveTableParams =
56+
| SaveTableParamsNew
57+
| SaveTableParamsDuplicate
58+
| SaveTableParamsExisting
59+
60+
type SaveTableParamsBase = {
61+
configuration: SaveTableConfiguration
62+
columns: ColumnField[]
63+
foreignKeyRelations: ForeignKey[]
64+
resolve: () => void
65+
}
66+
67+
type SaveTableParamsNew = SaveTableParamsBase & {
68+
action: 'create'
69+
payload: SaveTablePayloadNew
70+
}
71+
72+
type SaveTableParamsDuplicate = SaveTableParamsBase & {
73+
action: 'duplicate'
74+
payload: SaveTablePayloadDuplicate
75+
}
76+
77+
type SaveTableParamsExisting = SaveTableParamsBase & {
78+
action: 'update'
79+
payload: SaveTablePayloadExisting
80+
}
81+
82+
type SaveTablePayloadBase = {
83+
comment?: string | null
84+
}
85+
86+
type SaveTablePayloadNew = SaveTablePayloadBase & {
87+
name: string
88+
schema: string
89+
}
90+
91+
type SaveTablePayloadDuplicate = SaveTablePayloadBase & {
92+
name: string
93+
}
94+
95+
type SaveTablePayloadExisting = SaveTablePayloadBase & {
96+
name?: string
97+
rls_enabled?: boolean
98+
}
99+
100+
type SaveTableConfiguration = {
101+
tableId?: number
102+
importContent?: ImportContent
103+
isRLSEnabled: boolean
104+
isRealtimeEnabled: boolean
105+
isDuplicateRows: boolean
106+
existingForeignKeyRelations: ForeignKeyConstraint[]
107+
primaryKey?: Constraint
108+
}
109+
55110
export interface SidePanelEditorProps {
56111
editable?: boolean
57112
selectedTable?: PostgresTable
@@ -107,6 +162,8 @@ export const SidePanelEditor = ({
107162

108163
const getImpersonatedRoleState = useGetImpersonatedRoleState()
109164

165+
const isDuplicating = snap.sidePanel?.type === 'table' && snap.sidePanel.mode === 'duplicate'
166+
110167
const saveRow = async (
111168
payload: any,
112169
isNewRecord: boolean,
@@ -387,26 +444,11 @@ export const SidePanelEditor = ({
387444
}
388445
}
389446

390-
const saveTable = async (
391-
payload: {
392-
name: string
393-
schema: string
394-
comment?: string | null
395-
},
396-
columns: ColumnField[],
397-
foreignKeyRelations: ForeignKey[],
398-
isNewRecord: boolean,
399-
configuration: {
400-
tableId?: number
401-
importContent?: ImportContent
402-
isRLSEnabled: boolean
403-
isRealtimeEnabled: boolean
404-
isDuplicateRows: boolean
405-
existingForeignKeyRelations: ForeignKeyConstraint[]
406-
primaryKey?: Constraint
407-
},
408-
resolve: any
409-
) => {
447+
const saveTable = async (params: SaveTableParams) => {
448+
// action and payload are not destructured here to preserve type
449+
// narrowing later on
450+
const { configuration, columns, foreignKeyRelations, resolve } = params
451+
410452
let toastId
411453
let saveTableError = false
412454
const {
@@ -419,43 +461,14 @@ export const SidePanelEditor = ({
419461
} = configuration
420462

421463
try {
422-
if (
423-
snap.sidePanel?.type === 'table' &&
424-
snap.sidePanel.mode === 'duplicate' &&
425-
selectedTable
426-
) {
427-
const tableToDuplicate = selectedTable
428-
429-
toastId = toast.loading(`Duplicating table: ${tableToDuplicate.name}...`)
430-
431-
const table = await duplicateTable(project?.ref!, project?.connectionString, payload, {
432-
isRLSEnabled,
433-
isDuplicateRows,
434-
duplicateTable: tableToDuplicate,
435-
foreignKeyRelations,
436-
})
437-
if (isRealtimeEnabled) await updateTableRealtime(table, isRealtimeEnabled)
438-
439-
await Promise.all([
440-
queryClient.invalidateQueries({
441-
queryKey: tableKeys.list(project?.ref, table.schema, includeColumns),
442-
}),
443-
queryClient.invalidateQueries({ queryKey: entityTypeKeys.list(project?.ref) }),
444-
])
445-
446-
toast.success(
447-
`Table ${tableToDuplicate.name} has been successfully duplicated into ${table.name}!`,
448-
{ id: toastId }
449-
)
450-
onTableCreated(table)
451-
} else if (isNewRecord) {
452-
toastId = toast.loading(`Creating new table: ${payload.name}...`)
464+
if (params.action === 'create') {
465+
toastId = toast.loading(`Creating new table: ${params.payload.name}...`)
453466

454467
const table = await createTable({
455468
projectRef: project?.ref!,
456469
connectionString: project?.connectionString,
457470
toastId,
458-
payload,
471+
payload: params.payload,
459472
columns,
460473
foreignKeyRelations,
461474
isRLSEnabled,
@@ -473,15 +486,44 @@ export const SidePanelEditor = ({
473486

474487
toast.success(`Table ${table.name} is good to go!`, { id: toastId })
475488
onTableCreated(table)
476-
} else if (selectedTable) {
477-
toastId = toast.loading(`Updating table: ${selectedTable?.name}...`)
489+
} else if (params.action === 'duplicate' && !!selectedTable) {
490+
const tableToDuplicate = selectedTable
491+
toastId = toast.loading(`Duplicating table: ${tableToDuplicate.name}...`)
492+
493+
const table = await duplicateTable(
494+
project?.ref!,
495+
project?.connectionString,
496+
params.payload,
497+
{
498+
isRLSEnabled,
499+
isDuplicateRows,
500+
duplicateTable: tableToDuplicate,
501+
foreignKeyRelations,
502+
}
503+
)
504+
if (isRealtimeEnabled) await updateTableRealtime(table, isRealtimeEnabled)
505+
506+
await Promise.all([
507+
queryClient.invalidateQueries({
508+
queryKey: tableKeys.list(project?.ref, table.schema, includeColumns),
509+
}),
510+
queryClient.invalidateQueries({ queryKey: entityTypeKeys.list(project?.ref) }),
511+
])
512+
513+
toast.success(
514+
`Table ${tableToDuplicate.name} has been successfully duplicated into ${table.name}!`,
515+
{ id: toastId }
516+
)
517+
onTableCreated(table)
518+
} else if (params.action === 'update' && selectedTable) {
519+
toastId = toast.loading(`Updating table: ${selectedTable.name}...`)
478520

479521
const { table, hasError } = await updateTable({
480522
projectRef: project?.ref!,
481523
connectionString: project?.connectionString,
482524
toastId,
483525
table: selectedTable,
484-
payload,
526+
payload: params.payload,
485527
columns,
486528
foreignKeyRelations,
487529
existingForeignKeyRelations,
@@ -501,10 +543,10 @@ export const SidePanelEditor = ({
501543
`Table ${table.name} has been updated but there were some errors. Please check these errors separately.`
502544
)
503545
} else {
504-
if (ref && payload.name) {
546+
if (ref && params.payload.name) {
505547
// [Joshen] Only table entities can be updated via the dashboard
506548
const tabId = createTabId(ENTITY_TYPE.TABLE, { id: selectedTable.id })
507-
tabsSnap.updateTab(tabId, { label: payload.name })
549+
tabsSnap.updateTab(tabId, { label: params.payload.name })
508550
}
509551
toast.success(`Successfully updated ${table.name}!`, { id: toastId })
510552
}
@@ -632,7 +674,7 @@ export const SidePanelEditor = ({
632674
? selectedTable
633675
: undefined
634676
}
635-
isDuplicating={snap.sidePanel?.type === 'table' && snap.sidePanel.mode === 'duplicate'}
677+
isDuplicating={isDuplicating}
636678
templateData={
637679
snap.sidePanel?.type === 'table' && snap.sidePanel.templateData
638680
? {

apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/SidePanelEditor.utils.tsx

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ export const duplicateTable = async (
394394
connectionString,
395395
sql: [
396396
`CREATE TABLE "${sourceTableSchema}"."${duplicatedTableName}" (LIKE "${sourceTableSchema}"."${sourceTableName}" INCLUDING ALL);`,
397-
payload.comment !== undefined
397+
payload.comment != undefined
398398
? `comment on table "${sourceTableSchema}"."${duplicatedTableName}" is '${payload.comment}';`
399399
: '',
400400
].join('\n'),
@@ -724,7 +724,6 @@ export const updateTable = async ({
724724
const primaryKeyColumns = columns
725725
.filter((column) => column.isPrimaryKey)
726726
.map((column) => column.name)
727-
728727
const existingPrimaryKeyColumns = table.primary_keys.map((pk: PostgresPrimaryKey) => pk.name)
729728
const isPrimaryKeyUpdated = !isEqual(primaryKeyColumns, existingPrimaryKeyColumns)
730729

@@ -738,15 +737,16 @@ export const updateTable = async ({
738737
}
739738
}
740739

741-
// Update the table
742-
await updateTableMutation({
743-
projectRef,
744-
connectionString,
745-
id: table.id,
746-
name: table.name,
747-
schema: table.schema,
748-
payload,
749-
})
740+
if (Object.keys(payload).length > 0) {
741+
await updateTableMutation({
742+
projectRef,
743+
connectionString,
744+
id: table.id,
745+
name: table.name,
746+
schema: table.schema,
747+
payload,
748+
})
749+
}
750750

751751
// Track RLS enablement if it's being turned on
752752
if (payload.rls_enabled === true) {

0 commit comments

Comments
 (0)