mirror of
https://github.com/supabase/supabase.git
synced 2026-05-07 17:30:25 -04:00
Ali Waseem
a70264c8ad
## 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 -->
94 lines
3.1 KiB
TypeScript
94 lines
3.1 KiB
TypeScript
import { InformationCircleIcon } from '@heroicons/react/16/solid'
|
|
import { ArrowDown, ArrowUp } from 'lucide-react'
|
|
import { PropsWithChildren } from 'react'
|
|
import { cn, Tooltip, TooltipContent, TooltipTrigger } from 'ui'
|
|
|
|
export const QueryPanelContainer = ({
|
|
children,
|
|
className,
|
|
}: PropsWithChildren<{ className?: string }>) => (
|
|
<div className={cn('flex flex-col gap-y-0 py-4', className)}>{children}</div>
|
|
)
|
|
|
|
export const QueryPanelSection = ({
|
|
children,
|
|
className,
|
|
}: PropsWithChildren<{ className?: string }>) => (
|
|
<div className={cn('px-6 flex flex-col gap-y-0', className)}>{children}</div>
|
|
)
|
|
|
|
export const QueryPanelScoreSection = ({
|
|
className,
|
|
name,
|
|
description,
|
|
before: rawBefore,
|
|
after: rawAfter,
|
|
hideArrowMarkers = false,
|
|
}: {
|
|
className?: string
|
|
name: string
|
|
description: string
|
|
before?: number
|
|
after?: number
|
|
hideArrowMarkers?: boolean
|
|
}) => {
|
|
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>
|
|
<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>
|
|
)
|
|
}
|