From ce2ed77c02fa37578e02c795229f13875b6caca8 Mon Sep 17 00:00:00 2001 From: Joshen Lim Date: Wed, 19 Aug 2026 00:16:00 +0800 Subject: [PATCH] Add clickhouse migration banner to QueryEditor (#49184) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Context Adds the clickhouse migration banner into the QueryEditor for explorer if the source selected is logs - will apply for both the notebook query cells and query tab JFYI i've omitted out the diffing view for now, like what've currently got for the SQL Editor Got a separate ticket to look into that, but was thinking of waiting for [this PR](https://github.com/supabase/supabase/pull/49112) from Charis to go in first image image ## Summary by CodeRabbit * **New Features** * Added guidance in SQL editors to identify and rewrite legacy logs queries. * Integrated rewrite suggestions into the query editor’s existing SQL diff workflow. * Increased the height of embedded query editors for improved usability. * Kept rewrite guidance available when no rewrite is needed or an attempt is unsuccessful. * **Bug Fixes** * Improved spacing for empty query-result messages. * **Tests** * Added coverage for rewrite visibility, acceptance, dismissal, and no-change outcomes. --------- Co-authored-by: Ali Waseem --- .../QueryEditor/QueryResultRenderer.tsx | 4 +- .../interfaces/Explorer/QueryEditor/index.tsx | 44 +++-- .../SQLEditor/LegacyLogsRewriteBanner.tsx | 74 +++------ .../Logs/LegacyLogsRewriteBanner.test.tsx | 152 ++++++++++++++++++ .../Settings/Logs/LegacyLogsRewriteBanner.tsx | 62 +++++++ 5 files changed, 266 insertions(+), 70 deletions(-) create mode 100644 apps/studio/components/interfaces/Settings/Logs/LegacyLogsRewriteBanner.test.tsx create mode 100644 apps/studio/components/interfaces/Settings/Logs/LegacyLogsRewriteBanner.tsx diff --git a/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultRenderer.tsx b/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultRenderer.tsx index a75aa204ea0..79fbc5a407f 100644 --- a/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultRenderer.tsx +++ b/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultRenderer.tsx @@ -14,7 +14,7 @@ export const QueryResultRenderer = ({ result, view, chart }: QueryResultRenderer const { rows, error, autoLimit } = result ?? {} if (!result) { - return

Run the query to see results

+ return

Run the query to see results

} if (error) { @@ -22,7 +22,7 @@ export const QueryResultRenderer = ({ result, view, chart }: QueryResultRenderer } if ((rows ?? []).length === 0) { - return

Success. No rows returned

+ return

Success. No rows returned

} if (rows && rows.length > 0) { diff --git a/apps/studio/components/interfaces/Explorer/QueryEditor/index.tsx b/apps/studio/components/interfaces/Explorer/QueryEditor/index.tsx index 9812f213582..29b970fa242 100644 --- a/apps/studio/components/interfaces/Explorer/QueryEditor/index.tsx +++ b/apps/studio/components/interfaces/Explorer/QueryEditor/index.tsx @@ -23,6 +23,7 @@ import { type QueryDisplay, type QueryResult } from '../types' import { DisplaySettingsButton } from './DisplaySettingsButton' import { QueryResultRenderer } from './QueryResultRenderer' import { QuerySourceMenu } from './QuerySourceMenu' +import { LegacyLogsRewriteBanner } from '@/components/interfaces/Settings/Logs/LegacyLogsRewriteBanner' import { CodeEditor } from '@/components/ui/CodeEditor/CodeEditor' import { type DatabaseSourceParameters, @@ -256,24 +257,35 @@ export const QueryEditor = forwardRef(funct {showQuery && ( - - onSqlChange(value ?? '')} - onMount={(editor) => { - editor.onDidBlurEditorWidget(() => onSqlCommitRef.current?.(sqlRef.current)) + <> + sqlRef.current} + onProposal={({ modified }) => { + onSqlChange(modified) + onSqlCommit?.(modified) }} /> - + + onSqlChange(value ?? '')} + onMount={(editor) => { + editor.onDidBlurEditorWidget(() => onSqlCommitRef.current?.(sqlRef.current)) + }} + /> + + )} { diff: { isDiffOpen, setSourceSqlDiff, setSelectedDiffType }, } = useSqlEditorAssistant() - const isOtelLogsEnabled = useFlag('otelLegacyLogs') const snapV2 = useSqlEditorV2StateSnapshot() - - // The store is written on every keystroke, so debounce before running the - // dialect heuristics — the banner's visibility doesn't need per-character - // precision, and a settled value avoids flapping mid-edit. - const liveSql = snapV2.snippets[id]?.snippet.content?.unchecked_sql ?? '' - const settledSql = useDebounce(liveSql, LEGACY_LOGS_DIALECT_CHECK_DEBOUNCE_MS) - - const isLogsSnippetNeedingRewrite = useMemo( - () => - runSource._tag === 'logs' && - shouldOfferLegacyLogsRewrite({ sql: settledSql, isClickhouseLogsEnabled: isOtelLogsEnabled }), - [runSource._tag, settledSql, isOtelLogsEnabled] - ) - - const { state, requestRewrite, dismiss } = useLegacyLogsRewrite({ - // Rewrite exactly what's in the editor now, not the debounced value the - // visibility check used — they differ if the user clicked mid-edit. - readSql: () => getSqlEditorV2StateSnapshot().snippets[id]?.snippet.content?.unchecked_sql ?? '', - onProposal: ({ original, modified }) => { - setSourceSqlDiff({ original, modified }) - setSelectedDiffType(DiffType.Modification) - }, - }) - - // An outcome the user hasn't acknowledged stays up even once the query no longer - // looks legacy — otherwise a successful proposal would yank its own result away. - const hasUnacknowledgedOutcome = state.status === 'failed' || state.status === 'noRewriteNeeded' - const canShowBanner = !isDiffOpen && (isLogsSnippetNeedingRewrite || hasUnacknowledgedOutcome) - - if (!canShowBanner) return null + const sql = snapV2.snippets[id]?.snippet.content?.unchecked_sql ?? '' return ( - + + getSqlEditorV2StateSnapshot().snippets[id]?.snippet.content?.unchecked_sql ?? '' + } + onProposal={({ original, modified }) => { + setSourceSqlDiff({ original, modified }) + setSelectedDiffType(DiffType.Modification) + }} + hidden={isDiffOpen} + /> ) } diff --git a/apps/studio/components/interfaces/Settings/Logs/LegacyLogsRewriteBanner.test.tsx b/apps/studio/components/interfaces/Settings/Logs/LegacyLogsRewriteBanner.test.tsx new file mode 100644 index 00000000000..0afc4614b4e --- /dev/null +++ b/apps/studio/components/interfaces/Settings/Logs/LegacyLogsRewriteBanner.test.tsx @@ -0,0 +1,152 @@ +import { screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { platformComponents } from 'api-types' +import { FeatureFlagContext } from 'common' +import { http, HttpResponse } from 'msw' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { LegacyLogsRewriteBanner } from './LegacyLogsRewriteBanner' +import { API_URL } from '@/lib/constants' +import { createMockOrganizationResponse } from '@/tests/helpers' +import { customRender } from '@/tests/lib/custom-render' +import { addAPIMock, mswServer } from '@/tests/lib/msw' + +type ProjectDetailResponse = platformComponents['schemas']['ProjectDetailResponse'] + +const PROJECT_MOCK: ProjectDetailResponse = { + id: 1, + ref: 'default', + organization_id: 1, + name: 'Test Project', + status: 'ACTIVE_HEALTHY', + cloud_provider: 'AWS', + region: 'us-east-1', + db_host: 'db.default.supabase.co', + restUrl: 'https://default.supabase.co/rest/v1/', + inserted_at: '2024-01-01T00:00:00Z', + updated_at: '2024-01-01T00:00:00Z', + subscription_id: 'sub_123', + is_branch_enabled: false, + is_physical_backups_enabled: false, + high_availability: false, + integration_source: null, + connectionString: 'postgresql://postgres@localhost:5432/postgres', + is_hibernating: false, +} + +const ORG_MOCK = createMockOrganizationResponse({ slug: 'test-org', name: 'Test Org' }) + +const LEGACY_SQL = 'select event_message from edge_logs limit 10' + +const renderBanner = ({ + sql = LEGACY_SQL, + isLogsSource = true, + isOtelLogsEnabled = true, + hidden = false, + onProposal = vi.fn(), +}: { + sql?: string + isLogsSource?: boolean + isOtelLogsEnabled?: boolean + hidden?: boolean + onProposal?: (proposal: { original: string; modified: string }) => void +} = {}) => { + const utils = customRender( + + sql} + onProposal={onProposal} + hidden={hidden} + /> + + ) + return { ...utils, onProposal } +} + +describe('LegacyLogsRewriteBanner', () => { + beforeEach(() => { + addAPIMock({ method: 'get', path: '/platform/projects/:ref', response: PROJECT_MOCK }) + addAPIMock({ method: 'get', path: '/platform/organizations', response: [ORG_MOCK] }) + addAPIMock({ + method: 'post', + path: '/platform/projects/:ref/analytics/endpoints/logs.all.otel', + response: () => + HttpResponse.json({ result: [] }), + }) + }) + + it('offers a rewrite for legacy logs SQL when the source is logs and the flag is on', async () => { + renderBanner() + + expect( + await screen.findByText('Logs now run on a ClickHouse-backed engine') + ).toBeInTheDocument() + }) + + it('stays hidden when the source is not logs', () => { + renderBanner({ isLogsSource: false }) + + expect(screen.queryByText('Logs now run on a ClickHouse-backed engine')).not.toBeInTheDocument() + }) + + it('stays hidden when the SQL does not look legacy', () => { + renderBanner({ sql: 'select event_message from logs limit 10' }) + + expect(screen.queryByText('Logs now run on a ClickHouse-backed engine')).not.toBeInTheDocument() + }) + + it('stays hidden when the ClickHouse logs flag is off', () => { + renderBanner({ isOtelLogsEnabled: false }) + + expect(screen.queryByText('Logs now run on a ClickHouse-backed engine')).not.toBeInTheDocument() + }) + + it('stays hidden when a caller suppresses it via `hidden`, even for legacy SQL', () => { + renderBanner({ hidden: true }) + + expect(screen.queryByText('Logs now run on a ClickHouse-backed engine')).not.toBeInTheDocument() + }) + + it('hands an accepted rewrite to the caller via onProposal', async () => { + const rewritten = "select event_message from logs where source = 'edge_logs' limit 10" + mswServer.use( + http.post(`${API_URL}/ai/code/complete`, async () => HttpResponse.json(rewritten)) + ) + + const { onProposal } = renderBanner() + + await userEvent.click(await screen.findByRole('button', { name: 'Rewrite with Assistant' })) + + await waitFor(() => + expect(onProposal).toHaveBeenCalledWith({ original: LEGACY_SQL, modified: rewritten }) + ) + }) + + it('dismisses the offer', async () => { + renderBanner() + + await userEvent.click(await screen.findByRole('button', { name: 'Dismiss' })) + + await waitFor(() => + expect( + screen.queryByText('Logs now run on a ClickHouse-backed engine') + ).not.toBeInTheDocument() + ) + }) + + it('reports when the assistant found nothing to rewrite', async () => { + mswServer.use( + http.post(`${API_URL}/ai/code/complete`, async () => HttpResponse.json(LEGACY_SQL)) + ) + + renderBanner() + + await userEvent.click(await screen.findByRole('button', { name: 'Rewrite with Assistant' })) + + expect(await screen.findByText('No rewrite needed')).toBeInTheDocument() + }) +}) diff --git a/apps/studio/components/interfaces/Settings/Logs/LegacyLogsRewriteBanner.tsx b/apps/studio/components/interfaces/Settings/Logs/LegacyLogsRewriteBanner.tsx new file mode 100644 index 00000000000..61756661b0a --- /dev/null +++ b/apps/studio/components/interfaces/Settings/Logs/LegacyLogsRewriteBanner.tsx @@ -0,0 +1,62 @@ +import { useDebounce } from '@uidotdev/usehooks' +import { useFlag } from 'common' +import { useMemo } from 'react' + +import { LegacyLogsRewriteAdmonition } from './LegacyLogsRewriteAdmonition' +import { + LEGACY_LOGS_DIALECT_CHECK_DEBOUNCE_MS, + shouldOfferLegacyLogsRewrite, +} from '@/data/logs/logs-sql-rewrite' +import { + useLegacyLogsRewrite, + type LegacyLogsRewriteProposal, +} from '@/hooks/analytics/useLegacyLogsRewrite' + +export type LegacyLogsRewriteBannerProps = { + isLogsSource: boolean + sql: string + readSql: () => string + onProposal: (proposal: LegacyLogsRewriteProposal) => void + /** An extra hide condition layered on top of the offer's own visibility, for a + * surface that has its own reason to suppress it (the SQL editor while a diff + * is open). */ + hidden?: boolean +} + +/** + * Offers to rewrite a logs query still written in the old BigQuery dialect + * (per-service `FROM` tables, `unnest(metadata)` joins) to ClickHouse SQL. + * Shared by every surface that can run a logs query and wants to offer this — + * each one decides how to source `sql`/`readSql` and what an accepted + * `onProposal` does with the result (the SQL editor opens a diff, Explorer's + * query editor swaps the buffer directly). + */ +export const LegacyLogsRewriteBanner = ({ + isLogsSource, + sql, + readSql, + onProposal, + hidden, +}: LegacyLogsRewriteBannerProps) => { + const isOtelLogsEnabled = useFlag('otelLegacyLogs') + + const settledSql = useDebounce(sql, LEGACY_LOGS_DIALECT_CHECK_DEBOUNCE_MS) + + const needsRewrite = useMemo( + () => + isLogsSource && + shouldOfferLegacyLogsRewrite({ sql: settledSql, isClickhouseLogsEnabled: isOtelLogsEnabled }), + [isLogsSource, settledSql, isOtelLogsEnabled] + ) + + const { state, requestRewrite, dismiss } = useLegacyLogsRewrite({ readSql, onProposal }) + + const hasUnacknowledgedOutcome = state.status === 'failed' || state.status === 'noRewriteNeeded' + const isEligibleSurface = isLogsSource && isOtelLogsEnabled + + if (hidden || !isEligibleSurface || !(needsRewrite || hasUnacknowledgedOutcome)) return null + + return ( + + ) +}