mirror of
https://github.com/supabase/supabase.git
synced 2026-10-05 09:25:06 +03:00
fix(studio): index advisor recommendation column name (#43601)
## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Spotted by @kostasb, index advisor was recommending slightly different column names. Index advisor was running on mismatched queries thus recommending for the wrong table.
This commit is contained in:
1 parent
b997b7fb13
commit
3aa22f14c8
3 files changed
+73
-29
No files matched your search
@@ -1,15 +1,15 @@
|
||||
import { useQueryClient } from '@tanstack/react-query'
|
||||
import { createContext, useContext, PropsWithChildren, useState, useCallback } from 'react'
|
||||
|
||||
import { QueryIndexes } from 'components/interfaces/QueryPerformance/QueryIndexes'
|
||||
import { useIndexAdvisorStatus } from 'components/interfaces/QueryPerformance/hooks/useIsIndexAdvisorStatus'
|
||||
import { QueryIndexes } from 'components/interfaces/QueryPerformance/QueryIndexes'
|
||||
import { databaseKeys } from 'data/database/keys'
|
||||
import {
|
||||
useTableIndexAdvisorQuery,
|
||||
TableIndexAdvisorData,
|
||||
cleanIndexColumnName,
|
||||
IndexAdvisorSuggestion,
|
||||
TableIndexAdvisorData,
|
||||
useTableIndexAdvisorQuery,
|
||||
} from 'data/database/table-index-advisor-query'
|
||||
import { useSelectedProjectQuery } from 'hooks/misc/useSelectedProject'
|
||||
import { createContext, PropsWithChildren, useCallback, useContext, useState } from 'react'
|
||||
import { Sheet, SheetContent, SheetHeader, SheetTitle } from 'ui'
|
||||
|
||||
interface TableIndexAdvisorContextValue {
|
||||
@@ -80,7 +80,7 @@ export function TableIndexAdvisorProvider({
|
||||
suggestion.index_statements.some((stmt) => {
|
||||
const match = stmt.match(/USING\s+\w+\s*\(([^)]+)\)/i)
|
||||
if (match) {
|
||||
const columns = match[1].split(',').map((c) => c.trim().replace(/^"(.+)"$/, '$1'))
|
||||
const columns = match[1].split(',').map((c) => cleanIndexColumnName(c))
|
||||
return columns.includes(columnName)
|
||||
}
|
||||
return false
|
||||
@@ -101,6 +101,17 @@ export function TableIndexAdvisorProvider({
|
||||
// Get the first suggestion for the selected column to pass to QueryIndexes
|
||||
const selectedSuggestion = selectedColumn ? getSuggestionsForColumn(selectedColumn)[0] : null
|
||||
|
||||
const prefetchedIndexAdvisorResult = selectedSuggestion
|
||||
? {
|
||||
errors: [],
|
||||
index_statements: selectedSuggestion.index_statements,
|
||||
startup_cost_before: selectedSuggestion.startup_cost_before,
|
||||
startup_cost_after: selectedSuggestion.startup_cost_after,
|
||||
total_cost_before: selectedSuggestion.total_cost_before,
|
||||
total_cost_after: selectedSuggestion.total_cost_after,
|
||||
}
|
||||
: null
|
||||
|
||||
const value: TableIndexAdvisorContextValue = {
|
||||
isLoading,
|
||||
isAvailable: isIndexAdvisorAvailable,
|
||||
@@ -116,7 +127,7 @@ export function TableIndexAdvisorProvider({
|
||||
<TableIndexAdvisorContext.Provider value={value}>
|
||||
{children}
|
||||
<Sheet open={isSheetOpen} onOpenChange={(open) => !open && closeSheet()}>
|
||||
<SheetContent className="flex flex-col gap-0 p-0 sm:max-w-[500px]">
|
||||
<SheetContent className="flex flex-col gap-0 p-0 lg:!w-[calc(100vw-802px)] max-w-[700px]">
|
||||
<SheetHeader className="border-b px-5 py-3">
|
||||
<SheetTitle>Index Recommendation</SheetTitle>
|
||||
</SheetHeader>
|
||||
@@ -125,6 +136,7 @@ export function TableIndexAdvisorProvider({
|
||||
selectedRow={{ query: selectedSuggestion.query }}
|
||||
columnName={selectedColumn}
|
||||
suggestedSelectQuery={selectedSuggestion.query}
|
||||
prefetchedIndexAdvisorResult={prefetchedIndexAdvisorResult}
|
||||
onClose={closeSheet}
|
||||
/>
|
||||
)}
|
||||
|
||||
@@ -1,32 +1,35 @@
|
||||
import { Check, Table2, Lightbulb } from 'lucide-react'
|
||||
import { useState, useEffect } from 'react'
|
||||
|
||||
import { AccordionTrigger } from '@ui/components/shadcn/ui/accordion'
|
||||
import { useIndexAdvisorStatus } from 'components/interfaces/QueryPerformance/hooks/useIsIndexAdvisorStatus'
|
||||
import AlertError from 'components/ui/AlertError'
|
||||
import { DocsButton } from 'components/ui/DocsButton'
|
||||
import { Admonition } from 'ui-patterns'
|
||||
import { useDatabaseExtensionsQuery } from 'data/database-extensions/database-extensions-query'
|
||||
import { useGetIndexAdvisorResult } from 'data/database/retrieve-index-advisor-result-query'
|
||||
import {
|
||||
GetIndexAdvisorResultResponse,
|
||||
useGetIndexAdvisorResult,
|
||||
} from 'data/database/retrieve-index-advisor-result-query'
|
||||
import { useGetIndexesFromSelectQuery } from 'data/database/retrieve-index-from-select-query'
|
||||
import { useSelectedProjectQuery } from 'hooks/misc/useSelectedProject'
|
||||
import { DOCS_URL } from 'lib/constants'
|
||||
import { useTrack } from 'lib/telemetry/track'
|
||||
import { Check, Lightbulb, Table2 } from 'lucide-react'
|
||||
import { useEffect, useState } from 'react'
|
||||
import {
|
||||
Accordion_Shadcn_,
|
||||
AccordionContent_Shadcn_,
|
||||
AccordionItem_Shadcn_,
|
||||
Accordion_Shadcn_,
|
||||
Alert_Shadcn_,
|
||||
AlertDescription_Shadcn_,
|
||||
AlertTitle_Shadcn_,
|
||||
Alert_Shadcn_,
|
||||
Button,
|
||||
cn,
|
||||
CodeBlock,
|
||||
Collapsible_Shadcn_,
|
||||
CollapsibleContent_Shadcn_,
|
||||
CollapsibleTrigger_Shadcn_,
|
||||
Collapsible_Shadcn_,
|
||||
cn,
|
||||
} from 'ui'
|
||||
import { Admonition } from 'ui-patterns'
|
||||
import { GenericSkeletonLoader } from 'ui-patterns/ShimmeringLoader'
|
||||
|
||||
import { useIndexInvalidation } from './hooks/useIndexInvalidation'
|
||||
import { EnableIndexAdvisorButton } from './IndexAdvisor/EnableIndexAdvisorButton'
|
||||
import {
|
||||
@@ -34,15 +37,16 @@ import {
|
||||
createIndexes,
|
||||
hasIndexRecommendations,
|
||||
} from './IndexAdvisor/index-advisor.utils'
|
||||
import { QueryPerformanceRow } from './QueryPerformance.types'
|
||||
import { IndexAdvisorDisabledState } from './IndexAdvisor/IndexAdvisorDisabledState'
|
||||
import { IndexImprovementText } from './IndexAdvisor/IndexImprovementText'
|
||||
import { QueryPanelContainer, QueryPanelScoreSection, QueryPanelSection } from './QueryPanel'
|
||||
import { QueryPerformanceRow } from './QueryPerformance.types'
|
||||
|
||||
interface QueryIndexesProps {
|
||||
selectedRow: Pick<QueryPerformanceRow, 'query'>
|
||||
columnName?: string
|
||||
suggestedSelectQuery?: string
|
||||
prefetchedIndexAdvisorResult?: GetIndexAdvisorResultResponse | null
|
||||
|
||||
onClose?: () => void
|
||||
}
|
||||
@@ -54,6 +58,7 @@ export const QueryIndexes = ({
|
||||
selectedRow,
|
||||
columnName,
|
||||
suggestedSelectQuery,
|
||||
prefetchedIndexAdvisorResult,
|
||||
onClose,
|
||||
}: QueryIndexesProps) => {
|
||||
// [Joshen] TODO implement this logic once the linter rules are in
|
||||
@@ -83,22 +88,30 @@ export const QueryIndexes = ({
|
||||
|
||||
const { isIndexAdvisorEnabled } = useIndexAdvisorStatus()
|
||||
|
||||
const hasPrefetchedResult = prefetchedIndexAdvisorResult !== undefined
|
||||
|
||||
const {
|
||||
data: indexAdvisorResult,
|
||||
data: fetchedIndexAdvisorResult,
|
||||
error: indexAdvisorError,
|
||||
refetch,
|
||||
isError: isErrorIndexAdvisorResult,
|
||||
isSuccess: isSuccessIndexAdvisorResult,
|
||||
isLoading: isLoadingIndexAdvisorResult,
|
||||
isSuccess: isFetchSuccessIndexAdvisorResult,
|
||||
isLoading: isFetchLoadingIndexAdvisorResult,
|
||||
} = useGetIndexAdvisorResult(
|
||||
{
|
||||
projectRef: project?.ref,
|
||||
connectionString: project?.connectionString,
|
||||
query: selectedRow?.['query'],
|
||||
},
|
||||
{ enabled: isIndexAdvisorEnabled }
|
||||
{ enabled: isIndexAdvisorEnabled && !hasPrefetchedResult }
|
||||
)
|
||||
|
||||
const indexAdvisorResult = hasPrefetchedResult
|
||||
? prefetchedIndexAdvisorResult
|
||||
: fetchedIndexAdvisorResult
|
||||
const isSuccessIndexAdvisorResult = hasPrefetchedResult || isFetchSuccessIndexAdvisorResult
|
||||
const isLoadingIndexAdvisorResult = hasPrefetchedResult ? false : isFetchLoadingIndexAdvisorResult
|
||||
|
||||
const {
|
||||
index_statements,
|
||||
startup_cost_after,
|
||||
@@ -208,7 +221,9 @@ export const QueryIndexes = ({
|
||||
</div>
|
||||
</QueryPanelSection>
|
||||
)}
|
||||
<QueryPanelSection className="pt-2 mb-6">
|
||||
<QueryPanelSection
|
||||
className={cn('mb-6', !suggestedSelectQuery && !columnName ? 'pt-2' : 'pt-6')}
|
||||
>
|
||||
<div className="mb-4 flex flex-col gap-y-1">
|
||||
<h4 className="mb-2">Indexes in use</h4>
|
||||
<p className="text-sm text-foreground-light">
|
||||
|
||||
@@ -36,6 +36,10 @@ export function getTableIndexAdvisorSql(schema: string, table: string): string {
|
||||
const escapedSchema = schema.replace(/'/g, "''")
|
||||
const escapedTable = table.replace(/'/g, "''")
|
||||
|
||||
// Escape regex metacharacters so schema/table names are matched literally
|
||||
const regexSchema = escapedSchema.toLowerCase().replace(/[.+*?^${}()|[\]\\]/g, '\\$&')
|
||||
const regexTable = escapedTable.toLowerCase().replace(/[.+*?^${}()|[\]\\]/g, '\\$&')
|
||||
|
||||
return /* SQL */ `
|
||||
-- Get top 5 SELECT queries involving this table and run through index_advisor
|
||||
set search_path to public, extensions;
|
||||
@@ -48,14 +52,15 @@ with top_queries as (
|
||||
statements.mean_exec_time + statements.mean_plan_time as mean_time
|
||||
from pg_stat_statements as statements
|
||||
inner join pg_authid as auth on statements.userid = auth.oid
|
||||
where
|
||||
where
|
||||
-- Filter for SELECT queries only (index_advisor only works with SELECT)
|
||||
(lower(statements.query) like 'select%' or lower(statements.query) like 'with pgrst%')
|
||||
-- Filter for queries involving our table (handles schema.table and just table references)
|
||||
-- Filter for queries involving our table. Use regex word boundaries so that e.g.
|
||||
-- looking for table "orders" does not match queries on "orders_items".
|
||||
and (
|
||||
lower(statements.query) like '%${escapedSchema.toLowerCase()}.${escapedTable.toLowerCase()}%'
|
||||
or lower(statements.query) like '%from ${escapedTable.toLowerCase()}%'
|
||||
or lower(statements.query) like '%join ${escapedTable.toLowerCase()}%'
|
||||
lower(statements.query) ~ '(^|[^a-z0-9_$])${regexSchema}[.]${regexTable}($|[^a-z0-9_$])'
|
||||
or lower(statements.query) ~ '(^|[^a-z0-9_$])from[[:space:]]+${regexTable}($|[^a-z0-9_$])'
|
||||
or lower(statements.query) ~ '(^|[^a-z0-9_$])join[[:space:]]+${regexTable}($|[^a-z0-9_$])'
|
||||
)
|
||||
-- Exclude system queries
|
||||
and statements.query not like '%pg_catalog%'
|
||||
@@ -80,6 +85,17 @@ left join lateral (
|
||||
`.trim()
|
||||
}
|
||||
|
||||
// Strips ordering modifiers and outer quotes from a raw column token from an index statement.
|
||||
// e.g. '"created_at" DESC NULLS LAST' -> 'created_at'
|
||||
export function cleanIndexColumnName(raw: string): string {
|
||||
return raw
|
||||
.trim()
|
||||
.replace(/\s+(asc|desc)(\s+nulls\s+(first|last))?$/i, '')
|
||||
.replace(/\s+nulls\s+(first|last)$/i, '')
|
||||
.trim()
|
||||
.replace(/^"(.+)"$/, '$1')
|
||||
}
|
||||
|
||||
//Extracts column names from index statements
|
||||
//e.g. "CREATE INDEX ON public.users USING btree (email)" -> "email"
|
||||
function extractColumnsFromIndexStatements(indexStatements: string[]): string[] {
|
||||
@@ -92,8 +108,9 @@ function extractColumnsFromIndexStatements(indexStatements: string[]): string[]
|
||||
const columnPart = match[1]
|
||||
// Split by comma and clean up each column name
|
||||
columnPart.split(',').forEach((col) => {
|
||||
const cleanedCol = col.trim().replace(/^"(.+)"$/, '$1') // Remove quotes if present
|
||||
if (cleanedCol) {
|
||||
const cleanedCol = cleanIndexColumnName(col)
|
||||
// Only add simple identifiers — skip expressions like lower(col)
|
||||
if (cleanedCol && /^[a-z_][a-z0-9_$]*$/i.test(cleanedCol)) {
|
||||
columns.add(cleanedCol)
|
||||
}
|
||||
})
|
||||
|
||||
Reference in new issue
Block a user