mirror of
https://github.com/supabase/supabase.git
synced 2026-10-05 09:25:06 +03:00
fix(studio): coerce index advisor cost values to numbers (#44397)
## Summary - Coerces `before`/`after` cost values to `Number()` in `QueryPanelScoreSection` and `calculateImprovement` before any comparison or arithmetic - Fixes contradictory index advisor display where correct cost numbers showed 0% improvement and wrong arrow direction ## Root Cause When `index_advisor_result` is prefetched from the Reports SQL query (via `json_build_object`), cost values can arrive as strings instead of numbers. JavaScript string comparison is lexicographic, producing wrong results: | Expression | Numbers | Strings | |---|---|---| | `after > before` (arrow) | `50 > 100` → `false` ✅ | `"50" > "100"` → `true` ❌ | | `costBefore <= costAfter` (improvement calc) | `100 <= 50` → `false` ✅ | `"100" <= "50"` → `true` ❌ | The direct fetch path (`retrieve-index-advisor-result-query.ts`) validates through Zod and is unaffected. Only the prefetched path lacks validation. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved numeric value handling in query performance calculations to ensure more accurate and reliable improvement metrics. * **Refactor** * Enhanced type safety and numeric coercion for query performance score comparisons, resulting in more consistent and robust metric calculations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
1 parent
92692240bf
commit
a70264c8ad
2 files changed
+68
-61
No files matched your search
+9
-7
@@ -27,16 +27,18 @@ export function calculateImprovement(
|
||||
costBefore: number | undefined,
|
||||
costAfter: number | undefined
|
||||
): number {
|
||||
if (
|
||||
costBefore === undefined ||
|
||||
costAfter === undefined ||
|
||||
costBefore <= 0 ||
|
||||
costBefore <= costAfter
|
||||
) {
|
||||
if (costBefore === undefined || costAfter === undefined) {
|
||||
return 0
|
||||
}
|
||||
|
||||
return ((costBefore - costAfter) / costBefore) * 100
|
||||
const before = Number(costBefore)
|
||||
const after = Number(costAfter)
|
||||
|
||||
if (before <= 0 || before <= after) {
|
||||
return 0
|
||||
}
|
||||
|
||||
return ((before - after) / before) * 100
|
||||
}
|
||||
|
||||
interface CreateIndexParams {
|
||||
|
||||
@@ -21,8 +21,8 @@ export const QueryPanelScoreSection = ({
|
||||
className,
|
||||
name,
|
||||
description,
|
||||
before,
|
||||
after,
|
||||
before: rawBefore,
|
||||
after: rawAfter,
|
||||
hideArrowMarkers = false,
|
||||
}: {
|
||||
className?: string
|
||||
@@ -31,58 +31,63 @@ export const QueryPanelScoreSection = ({
|
||||
before?: number
|
||||
after?: number
|
||||
hideArrowMarkers?: boolean
|
||||
}) => (
|
||||
<div className={cn('py-4 px-4 flex', className)}>
|
||||
<div className="flex gap-x-2 w-48">
|
||||
<span className="text-sm">{name}</span>
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild className="mt-1">
|
||||
<InformationCircleIcon className="transition text-foreground-muted w-3 h-3 data-[state=delayed-open]:text-foreground-light" />
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="top" className="w-52 text-center">
|
||||
{description}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
</div>
|
||||
<div className="flex flex-col gap-y-1">
|
||||
<div className="flex gap-x-2 text-sm">
|
||||
<span className="text-foreground-light w-20">Currently:</span>
|
||||
<span
|
||||
className={cn(
|
||||
'font-mono',
|
||||
before !== undefined && after !== undefined && before !== after
|
||||
? 'text-foreground-light'
|
||||
: ''
|
||||
)}
|
||||
>
|
||||
{before}
|
||||
</span>
|
||||
}) => {
|
||||
const before = rawBefore !== undefined ? Number(rawBefore) : undefined
|
||||
const after = rawAfter !== undefined ? Number(rawAfter) : undefined
|
||||
|
||||
return (
|
||||
<div className={cn('py-4 px-4 flex', className)}>
|
||||
<div className="flex gap-x-2 w-48">
|
||||
<span className="text-sm">{name}</span>
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild className="mt-1">
|
||||
<InformationCircleIcon className="transition text-foreground-muted w-3 h-3 data-[state=delayed-open]:text-foreground-light" />
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="top" className="w-52 text-center">
|
||||
{description}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
</div>
|
||||
{before !== undefined && after !== undefined && before !== after && (
|
||||
<div className="flex items-center gap-x-2 text-sm">
|
||||
<span className="text-foreground-light w-20">With index:</span>
|
||||
<span className="font-mono">{after}</span>
|
||||
{before !== undefined && !hideArrowMarkers && (
|
||||
<div className="flex items-center gap-x-1">
|
||||
{after > before ? (
|
||||
<ArrowUp size={14} className="text-warning" />
|
||||
) : (
|
||||
<ArrowDown size={14} className="text-brand" />
|
||||
)}
|
||||
{typeof before === 'number' && before !== 0 && !isNaN(before) && isFinite(before) && (
|
||||
<span
|
||||
className={cn(
|
||||
'font-mono tracking-tighter',
|
||||
after > before ? 'text-warning' : 'text-brand'
|
||||
)}
|
||||
>
|
||||
{(((before - after) / before) * 100).toFixed(2)}%
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
<div className="flex flex-col gap-y-1">
|
||||
<div className="flex gap-x-2 text-sm">
|
||||
<span className="text-foreground-light w-20">Currently:</span>
|
||||
<span
|
||||
className={cn(
|
||||
'font-mono',
|
||||
before !== undefined && after !== undefined && before !== after
|
||||
? 'text-foreground-light'
|
||||
: ''
|
||||
)}
|
||||
>
|
||||
{before}
|
||||
</span>
|
||||
</div>
|
||||
)}
|
||||
{before !== undefined && after !== undefined && before !== after && (
|
||||
<div className="flex items-center gap-x-2 text-sm">
|
||||
<span className="text-foreground-light w-20">With index:</span>
|
||||
<span className="font-mono">{after}</span>
|
||||
{before !== undefined && !hideArrowMarkers && (
|
||||
<div className="flex items-center gap-x-1">
|
||||
{after > before ? (
|
||||
<ArrowUp size={14} className="text-warning" />
|
||||
) : (
|
||||
<ArrowDown size={14} className="text-brand" />
|
||||
)}
|
||||
{before !== 0 && !isNaN(before) && isFinite(before) && (
|
||||
<span
|
||||
className={cn(
|
||||
'font-mono tracking-tighter',
|
||||
after > before ? 'text-warning' : 'text-brand'
|
||||
)}
|
||||
>
|
||||
{(((before - after) / before) * 100).toFixed(2)}%
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
)
|
||||
}
|
||||
Reference in new issue
Block a user