From 75f1e5e60e206e924115e6851de1ead6f209fb07 Mon Sep 17 00:00:00 2001 From: huyplb Date: Tue, 8 Sep 2026 18:39:06 -0600 Subject: [PATCH] Reuse what the repo already had, and fix two things the review found wrong MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A four-angle review of the last three merged PRs. Two findings were defects, not untidiness. **SQL Server reported size and rows multiplied by the column count.** The probe's scalars were `SUM(...) OVER ()` beside a `LEFT JOIN sys.columns`, which fans the result to one row per column; the window then summed the fanned set. A 20-column table reported 20x its rows and 20x its size, and `size_bytes` — the figure a doc comment in this very diff calls "the kind of figure people act on". The row count had the defect already and the new size line copied its shape. Both now come from a CROSS APPLY that aggregates on its own row set. Size counts every partition, since the hint promises table *and* indexes; rows count only index_id 0/1, or every index would multiply them again. My test asserted `toContain('used_page_count')`, which could never have caught this. **The orphan query was invalid on Oracle.** It aliased tables with `AS`, and Oracle answers ORA-03048. Verified both ways on Oracle 23: with `AS`, the error; without it, the query runs. A bare correlation name is accepted everywhere this ships, so no dialect branch — one spelling, and a test across five dialects. Reuse, which is what the review was for: - `formatBytes` already existed in @foxschema/sql, imported by three sibling components. I had written a fourth copy, worse than the original — it took `number` where the shared one takes `number | null | undefined`, so I also added a null guard at the call site that the real one makes unnecessary, and tests for behaviour dba-utilities.test.ts already covered. All deleted. - Row counts used bare `.toLocaleString()`, which follows the host locale, two clicks from a schema explorer that uses `formatRowCount` and pins en-US. - The FK identity key was spelled three times, once with `', '` against `','`. The two Peek copies had to stay byte-identical or a count written under one key was never found under the other. Now `fkKey`. - `foreignKeys` re-implemented `findCachedTable` from a module the file already imports — and more weakly, matching a bare name across schemas. - The new per-FK row wrote out `panelCls` plus its exact padding, in the file that imports `Panel`. That is the clearest evidence the primitive was not yet the easy path. Efficiency: the orphan check ran one request per foreign key, serially, and /sql/execute opens and closes a connection per request — so six keys meant six handshakes paid one after another, and one failure discarded every scan already paid for. `executeSql` already takes an array and runs it on one connection with per-statement isolation, so it is now a single request that keeps the counts that succeeded. Verified live: two orphan statements in one request return 1 and 1 over planted data. Unbounded Promise.all would have been worse than either, since each call creates its own connection. Also: `pg_total_relation_size` was in the target list of a query returning one row per column, and each evaluation stats every fork, the TOAST relation and every index. It is now an uncorrelated scalar subquery, evaluated once. Smaller: narrowed two whole-store selectors that re-ran the catalog probe on unrelated connections; memoised AccountStage's two derivations, which re-ran on every keystroke in the principal filter; made its props required and deleted an unreachable branch; dropped `Panel`'s `padded`, which had no caller outside the test written for it; removed the `sql.raw` escape hatch from the orphan builder in favour of a closed shape parameter. Suite 3655 passed, eslint 0 errors, typecheck clean. Co-Authored-By: Claude Opus 5 --- .../frontend/app/store/useSqlEditorStore.ts | 3 +- .../components/AccessPermissionPanel.tsx | 38 ++++-- .../components/PeekInsight.test.tsx | 26 +--- .../sql-editor/components/PeekInsight.tsx | 128 ++++++++++-------- .../shared/components/surfaces.test.tsx | 13 -- .../frontend/shared/components/surfaces.tsx | 10 +- .../shared/lib/tablePreview.orphans.test.ts | 11 ++ .../src/frontend/shared/lib/tablePreview.ts | 26 +++- .../sqlServer/sqlserver.table-insight.ts | 25 +++- 9 files changed, 149 insertions(+), 131 deletions(-) diff --git a/apps/web/src/frontend/app/store/useSqlEditorStore.ts b/apps/web/src/frontend/app/store/useSqlEditorStore.ts index 001e69b5..6639af67 100644 --- a/apps/web/src/frontend/app/store/useSqlEditorStore.ts +++ b/apps/web/src/frontend/app/store/useSqlEditorStore.ts @@ -28,6 +28,7 @@ import { buildSampleBookmarks } from '@/features/sql-editor/lib/sqlEditorSamples import { buildForeignKeyDrilldown, buildOrphanPeek, + fkKey, buildRowLookup, buildTablePreview, composePeekSql, @@ -1849,7 +1850,7 @@ export const useSqlEditorStore = create()( if ('error' in composed) return; const entry: DataPeekEntry = { id: `peek-${Date.now()}-${Math.random().toString(36).slice(2, 7)}`, - title: `${childTable} · orphans via ${fk.name || (fk.columns ?? []).join(', ')}`, + title: `${childTable} · orphans via ${fkKey(fk)}`, tableName: childTable, baseSql: built.sql, baseParams: built.params, diff --git a/apps/web/src/frontend/features/access/components/AccessPermissionPanel.tsx b/apps/web/src/frontend/features/access/components/AccessPermissionPanel.tsx index 1d75f21e..989df8c0 100644 --- a/apps/web/src/frontend/features/access/components/AccessPermissionPanel.tsx +++ b/apps/web/src/frontend/features/access/components/AccessPermissionPanel.tsx @@ -573,15 +573,25 @@ export const AccessPermissionPanel: React.FC<{ const AccountStage: React.FC<{ principal: DbPrincipal; /** The catalog's privileges, so a drop can be described before it is run. */ - privileges?: readonly DbPrivilege[]; - dialect?: string; + privileges: readonly DbPrivilege[]; + dialect: string; onManageUsers?: () => void; -}> = ({ principal, privileges = [], dialect, onManageUsers }) => { - const support = dialect ? userManagementSupport(dialect) : null; - const alterations = support - ? availableAlterations(support, principal.kind === 'user' ? 'user' : 'role') - : []; - const dropNotes = dropSafetyNotes(principal, privileges); +}> = ({ principal, privileges, dialect, onManageUsers }) => { + // Both memoised: this is an unmemoised child of a panel that owns the + // principal filter, so every keystroke re-ran dropSafetyNotes, which walks + // the whole privileges array. + const alterations = useMemo( + () => + availableAlterations( + userManagementSupport(dialect), + principal.kind === 'user' ? 'user' : 'role' + ), + [dialect, principal.kind] + ); + const dropNotes = useMemo( + () => dropSafetyNotes(principal, privileges), + [principal, privileges] + ); const login = principal.canLogin === true ? 'Can log in' @@ -615,9 +625,7 @@ const AccountStage: React.FC<{ Alteration {alterations.length === 0 ? (

- {support - ? 'This engine has no edit actions for this kind of account.' - : 'Choose a connection to see what this engine can change.'} + This engine has no edit actions for this kind of account.

) : (
    @@ -659,9 +667,9 @@ const AccountStage: React.FC<{ )}
    -

    + Member of -

    + {principal.memberOf.length === 0 ? (

    Not a member of any role.

    ) : ( @@ -678,9 +686,9 @@ const AccountStage: React.FC<{ )}
    -

    + Members -

    + {principal.members.length === 0 ? (

    No members.

    ) : ( diff --git a/apps/web/src/frontend/features/sql-editor/components/PeekInsight.test.tsx b/apps/web/src/frontend/features/sql-editor/components/PeekInsight.test.tsx index 6b2a6ce2..49b94bcf 100644 --- a/apps/web/src/frontend/features/sql-editor/components/PeekInsight.test.tsx +++ b/apps/web/src/frontend/features/sql-editor/components/PeekInsight.test.tsx @@ -5,7 +5,7 @@ */ import { describe, expect, it, vi, beforeEach } from 'vitest'; import { render, screen, waitFor } from '@testing-library/react'; -import { PeekInsight, formatBytes } from './PeekInsight'; +import { PeekInsight } from './PeekInsight'; const fetchTableInsight = vi.fn(); vi.mock('@/shared/api/schemaApi', () => ({ @@ -55,27 +55,3 @@ describe('PeekInsight', () => { expect(screen.getByTestId('data-peek-insight-col-id')).toBeTruthy(); }); }); - -describe('formatBytes', () => { - it('keeps small tables in bytes rather than rounding them to 0 KB', () => { - expect(formatBytes(0)).toBe('0 B'); - expect(formatBytes(1023)).toBe('1023 B'); - }); - - it('shows one decimal under 10 so 1.2 GB does not read as 1 GB', () => { - expect(formatBytes(1024 * 1024 * 1024 * 1.25)).toBe('1.3 GB'); - expect(formatBytes(1536)).toBe('1.5 KB'); - }); - - it('drops the decimal above 10, where it is noise against the rounding already there', () => { - expect(formatBytes(1024 * 1024 * 890)).toBe('890 MB'); - }); - - it('refuses to invent a size from a nonsense figure', () => { - // A negative or non-finite size means the catalog answered with something - // this code does not understand. "—" says that; "0 B" would claim the - // table is empty. - expect(formatBytes(-1)).toBe('—'); - expect(formatBytes(Number.NaN)).toBe('—'); - }); -}) diff --git a/apps/web/src/frontend/features/sql-editor/components/PeekInsight.tsx b/apps/web/src/frontend/features/sql-editor/components/PeekInsight.tsx index aba32dd4..40d21a8f 100644 --- a/apps/web/src/frontend/features/sql-editor/components/PeekInsight.tsx +++ b/apps/web/src/frontend/features/sql-editor/components/PeekInsight.tsx @@ -9,9 +9,18 @@ import React, { useEffect, useMemo, useState } from 'react'; import { Loader2 } from 'lucide-react'; import { fetchTableInsight, type TableInsightResponse } from '@/shared/api/schemaApi'; -import { tableNameParts, buildOrphanCount, fkDrillTableName } from '@/shared/lib/tablePreview'; +import type { PreviewQuery } from '@/shared/lib/tablePreview'; +import { + buildOrphanCount, + fkDrillTableName, + fkKey, + findCachedTable, + tableNameParts, +} from '@/shared/lib/tablePreview'; +import { formatBytes, formatRowCount } from '@foxschema/sql'; import { useSqlEditorStore } from '@/app/store/useSqlEditorStore'; -import { StatCard } from '@/shared/components/surfaces'; +import { useSyncStore } from '@/app/store/useSyncStore'; +import { Panel, StatCard } from '@/shared/components/surfaces'; import { executeSql } from '@/shared/api/sqlApi'; function tableRef(tableName: string, fallbackSchema?: string): { table: string; schema?: string } { @@ -27,29 +36,15 @@ function pct(frac: number | null | undefined): string { return `${Math.round(frac * 1000) / 10}%`; } - -/** Bytes as a person reads them. Binary units, since that is what catalogs report. */ -export function formatBytes(bytes: number): string { - if (!Number.isFinite(bytes) || bytes < 0) return '—'; - if (bytes < 1024) return `${bytes} B`; - const units = ['KB', 'MB', 'GB', 'TB', 'PB']; - let value = bytes / 1024; - let unit = 0; - while (value >= 1024 && unit < units.length - 1) { - value /= 1024; - unit += 1; - } - // One decimal below 10 so 1.2 GB does not collapse to 1 GB, none above it - // where the extra digit is noise against the rounding already in the number. - return `${value < 10 ? value.toFixed(1) : Math.round(value)} ${units[unit]}`; -} - export const PeekInsight: React.FC<{ connectionId: string; tableName: string; schema?: string; }> = ({ connectionId, tableName, schema }) => { - const sessionPasswords = useSqlEditorStore((s) => s.sessionPasswords); + // Narrow selectors: the whole `password` map and the whole + // `schemaCache` object get replaced on activity for *other* connections, and + // depending on them re-ran the catalog probe and re-walked every table. + const password = useSqlEditorStore((s) => s.sessionPasswords[connectionId]); const [data, setData] = useState(null); const [error, setError] = useState(null); const [loading, setLoading] = useState(true); @@ -57,26 +52,29 @@ export const PeekInsight: React.FC<{ const [checkingOrphans, setCheckingOrphans] = useState(false); const [orphanError, setOrphanError] = useState(null); const openDataPeekOrphans = useSqlEditorStore((st) => st.openDataPeekOrphans); - const schemaCache = useSqlEditorStore((st) => st.schemaCache); + const cachedTables = useSqlEditorStore((st) => st.schemaCache[connectionId]?.tables); + // From the connection, the same source openDataPeekOrphans uses. Reading it + // off the insight response instead let the count and the drill-down be built + // for different dialects — the response's `dialect` is optional, so a missing + // one silently fell back to default quoting. + const dialect = useSyncStore((st) => st.connections.find((c) => c.id === connectionId)?.dialect ?? ''); + /** Foreign keys declared on this table, from the catalog already in hand. */ /** Foreign keys declared on this table, from the catalog already in hand. */ const foreignKeys = useMemo(() => { - const ref = tableRef(tableName, schema); - const tables = schemaCache[connectionId]?.tables ?? []; - const match = tables.find((t) => { - const parts = tableNameParts(t.name); - return (parts[parts.length - 1] ?? t.name).toLowerCase() === ref.table.toLowerCase(); - }); + // findCachedTable, not a local match: it tries the fully-qualified name + // before falling back to the bare one, so `public.orders` cannot silently + // resolve to `other_schema.orders`. + const match = findCachedTable(cachedTables, tableName); return (match?.foreignKeys ?? []).filter( (fk) => (fk.columns ?? []).length > 0 && (fk.referencedColumns ?? []).length > 0 ); - }, [schemaCache, connectionId, tableName, schema]); + }, [cachedTables, tableName]); /** Total across every FK, or null while nobody has asked. */ - const orphanTotal = useMemo( - () => (orphanCounts ? Object.values(orphanCounts).reduce((a, b) => a + b, 0) : null), - [orphanCounts] - ); + const orphanTotal = orphanCounts + ? Object.values(orphanCounts).reduce((a, b) => a + b, 0) + : null; // The insight above is catalog-only and constant time. This is a scan of the // child against each parent, so it runs when asked and not before. @@ -85,27 +83,37 @@ export const PeekInsight: React.FC<{ setCheckingOrphans(true); setOrphanError(null); try { - const dialect = data?.dialect ?? ''; + // One request carrying every statement, not one request per key. + // /sql/execute opens a connection, runs the statements on it, and closes + // it (sql-execute.service.ts), so N calls meant N connect/auth handshakes + // paid serially. Batching also isolates failures per statement, where the + // old loop threw away counts already paid for as soon as one key failed. + const built = foreignKeys + .map((fk) => ({ fk, q: buildOrphanCount(tableName, fk, dialect) })) + .filter((x): x is { fk: (typeof foreignKeys)[number]; q: PreviewQuery } => x.q != null); + if (built.length === 0) return; + const { results } = await executeSql( + { connectionId, password: password || undefined }, + built.map((b) => b.q.sql), + undefined, + undefined, + built.map((b) => b.q.params) + ); const counts: Record = {}; - for (const fk of foreignKeys) { - const built = buildOrphanCount(tableName, fk, dialect); - if (!built) continue; - const { results } = await executeSql( - { connectionId, password: sessionPasswords[connectionId] || undefined }, - [built.sql], - undefined, - undefined, - [built.params] - ); - const first = results[0]; - if (!first || !first.ok) { - setOrphanError(first && 'error' in first ? String(first.error) : 'Orphan check failed.'); + const failures: string[] = []; + built.forEach((b, i) => { + const r = results[i]; + if (!r || !r.ok) { + failures.push(r && 'error' in r ? String(r.error) : 'failed'); return; } - counts[fk.name || (fk.columns ?? []).join(',')] = Number(first.rows?.[0]?.[0] ?? 0); - } - setOrphanCounts(counts); + counts[fkKey(b.fk)] = Number(r.rows?.[0]?.[0] ?? 0); + }); + // Keep whatever succeeded; a broken key should not hide the others. + setOrphanCounts(Object.keys(counts).length > 0 ? counts : null); + setOrphanError(failures.length > 0 ? failures.join(' · ') : null); } catch (e) { + setOrphanCounts(null); setOrphanError(e instanceof Error ? e.message : String(e)); } finally { setCheckingOrphans(false); @@ -118,7 +126,7 @@ export const PeekInsight: React.FC<{ setLoading(true); setError(null); void fetchTableInsight( - { connectionId, password: sessionPasswords[connectionId] || undefined }, + { connectionId, password: password || undefined }, { table: ref.table, schema: ref.schema } ) .then((res) => { @@ -136,7 +144,7 @@ export const PeekInsight: React.FC<{ return () => { cancelled = true; }; - }, [connectionId, tableName, schema, sessionPasswords]); + }, [connectionId, tableName, schema, password]); const cards = useMemo(() => { if (!data) return null; @@ -178,13 +186,13 @@ export const PeekInsight: React.FC<{ {(fk.columns ?? []).join(', ')} → {fkDrillTableName(fk)} @@ -252,7 +260,7 @@ export const PeekInsight: React.FC<{ count > 0 ? 'text-rose-300' : 'text-emerald-300' }`} > - {count === 0 ? 'no orphans' : `${count.toLocaleString()} orphans`} + {count === 0 ? 'no orphans' : `${formatRowCount(count)} orphans`} {matched && {matched}} @@ -267,7 +275,7 @@ export const PeekInsight: React.FC<{ Peek orphans )} -
    + ); })}
    @@ -295,7 +303,7 @@ export const PeekInsight: React.FC<{

    Estimated rows:{' '} - {data.estimatedRows == null ? '—' : data.estimatedRows.toLocaleString()} + {formatRowCount(data.estimatedRows)} {data.support?.hint ? ( {data.support.hint} diff --git a/apps/web/src/frontend/shared/components/surfaces.test.tsx b/apps/web/src/frontend/shared/components/surfaces.test.tsx index f9b414fd..9e90d9d0 100644 --- a/apps/web/src/frontend/shared/components/surfaces.test.tsx +++ b/apps/web/src/frontend/shared/components/surfaces.test.tsx @@ -67,16 +67,3 @@ describe('StatCard', () => { expect(screen.getByTestId('zero').textContent).toContain('0'); }); }); - -describe('Panel', () => { - it('can drop its padding for callers that own their own spacing', () => { - const { rerender } = render(x); - expect(screen.getByTestId('p').className).toContain('px-2.5'); - rerender( - - x - - ); - expect(screen.getByTestId('p').className).not.toContain('px-2.5'); - }); -}); diff --git a/apps/web/src/frontend/shared/components/surfaces.tsx b/apps/web/src/frontend/shared/components/surfaces.tsx index d99a41bd..03270b6b 100644 --- a/apps/web/src/frontend/shared/components/surfaces.tsx +++ b/apps/web/src/frontend/shared/components/surfaces.tsx @@ -34,15 +34,11 @@ export const SectionLabel: React.FC<{ export const Panel: React.FC<{ children: React.ReactNode; - /** Extra classes; padding is included by default so callers rarely need it. */ + /** Extra classes. Padding is part of the treatment, not a choice. */ className?: string; - padded?: boolean; testId?: string; -}> = ({ children, className, padded = true, testId }) => ( -

    +}> = ({ children, className, testId }) => ( +
    {children}
    ); diff --git a/apps/web/src/frontend/shared/lib/tablePreview.orphans.test.ts b/apps/web/src/frontend/shared/lib/tablePreview.orphans.test.ts index 952702ad..f41f053a 100644 --- a/apps/web/src/frontend/shared/lib/tablePreview.orphans.test.ts +++ b/apps/web/src/frontend/shared/lib/tablePreview.orphans.test.ts @@ -79,6 +79,17 @@ describe('buildOrphanCount', () => { expect(q.sql).toContain('"reference"."customers"'); }); + it('aliases tables without AS, which Oracle rejects', () => { + // Verified against Oracle 23: `FROM demo_a.orders AS fox_c` is ORA-03048, + // "SQL reserved word 'AS' is not syntactically valid". A bare correlation + // name is accepted by every engine here, so there is no dialect branch. + for (const dialect of ['oracle', 'postgres', 'mysql', 'sqlserver', 'db2']) { + const q = buildOrphanCount('demo_a.orders', fk(), dialect)!; + expect(q.sql, dialect).not.toMatch(/\bAS\s+fox_[cp]\b/); + expect(q.sql, dialect).toMatch(/fox_c\b/); + } + }); + it('offers nothing when the catalog gave no usable column pair', () => { expect(buildOrphanCount('public.orders', fk({ columns: [] }), 'postgres')).toBeNull(); expect( diff --git a/apps/web/src/frontend/shared/lib/tablePreview.ts b/apps/web/src/frontend/shared/lib/tablePreview.ts index 172d957f..6592563b 100644 --- a/apps/web/src/frontend/shared/lib/tablePreview.ts +++ b/apps/web/src/frontend/shared/lib/tablePreview.ts @@ -105,6 +105,18 @@ export function buildRowLookup( return { sql: text, params }; } +/** + * How a foreign key is named, both as a map key and on screen. + * + * Spelled in three places before this — twice in Peek Insight and once in the + * store — and the store's copy joined with `', '` where the others used `','`. + * The two Peek copies had to stay byte-identical or the count written under one + * key was never found under the other, showing "not checked" for every key. + */ +export function fkKey(fk: Pick): string { + return fk.name || (fk.columns ?? []).join(','); +} + /** * Child rows whose foreign key points at a parent row that is not there. * @@ -127,7 +139,7 @@ export function buildRowLookup( * leave the affordance off rather than offer a check that cannot run. */ function orphanQuery( - select: string, + shape: 'count' | 'rows', childTable: string, fk: ForeignKeyInfo, dialect: string @@ -141,12 +153,16 @@ function orphanQuery( if (childParts.length === 0 || parentParts.length === 0) return null; // Aliases keep the two sides apart when a table references itself. - let query = sql`SELECT ${sql.raw(select)} FROM ${sql.id(...childParts)} AS fox_c WHERE `; + // No `AS` before a table alias: Oracle rejects it (ORA-00933), while a bare + // correlation name is valid on every engine this ships against. One spelling + // beats a dialect branch for a difference this small. + const select = shape === 'count' ? sql`COUNT(*) AS fox_orphans` : sql`fox_c.*`; + let query = sql`SELECT ${select} FROM ${sql.id(...childParts)} fox_c WHERE `; childCols.forEach((col, i) => { const joiner = i === 0 ? sql`` : sql` AND `; query = sql`${query}${joiner}fox_c.${sql.id(col)} IS NOT NULL`; }); - query = sql`${query} AND NOT EXISTS (SELECT 1 FROM ${sql.id(...parentParts)} AS fox_p WHERE `; + query = sql`${query} AND NOT EXISTS (SELECT 1 FROM ${sql.id(...parentParts)} fox_p WHERE `; refCols.forEach((refCol, i) => { const joiner = i === 0 ? sql`` : sql` AND `; query = sql`${query}${joiner}fox_p.${sql.id(refCol)} = fox_c.${sql.id(childCols[i]!)}`; @@ -163,7 +179,7 @@ export function buildOrphanCount( fk: ForeignKeyInfo, dialect: string ): PreviewQuery | null { - return orphanQuery('COUNT(*) AS fox_orphans', childTable, fk, dialect); + return orphanQuery('count', childTable, fk, dialect); } /** The orphan rows themselves — what "Peek orphans" opens. */ @@ -172,7 +188,7 @@ export function buildOrphanPeek( fk: ForeignKeyInfo, dialect: string ): PreviewQuery | null { - return orphanQuery('fox_c.*', childTable, fk, dialect); + return orphanQuery('rows', childTable, fk, dialect); } export interface FkColumnLink { diff --git a/packages/sql/src/providers/sqlServer/sqlserver.table-insight.ts b/packages/sql/src/providers/sqlServer/sqlserver.table-insight.ts index 4cd9ce82..3830fa42 100644 --- a/packages/sql/src/providers/sqlServer/sqlserver.table-insight.ts +++ b/packages/sql/src/providers/sqlServer/sqlserver.table-insight.ts @@ -23,13 +23,28 @@ SELECT c.name AS column_name, NULL AS n_distinct, NULL AS null_frac, - SUM(p.row_count) OVER () AS estimated_rows, - -- SQL Server pages are always 8 KB, so page count converts to bytes exactly. - (SUM(p.used_page_count) OVER () * 8192) AS size_bytes + sz.estimated_rows, + sz.size_bytes FROM sys.tables t INNER JOIN sys.schemas s ON s.schema_id = t.schema_id -INNER JOIN sys.dm_db_partition_stats p - ON p.object_id = t.object_id AND p.index_id IN (0, 1) +-- Table-level totals, computed once per table. +-- +-- These used to be SUM(...) OVER () beside the LEFT JOIN to sys.columns, which +-- fans the result out to one row per column: the window then summed the fanned +-- set and every scalar came back multiplied by the column count. A 20-column +-- table reported 20x its rows and 20x its size. CROSS APPLY keeps the +-- aggregate on its own row set, where the column join cannot reach it. +CROSS APPLY ( + SELECT + -- Rows live only in the heap or clustered index; counting every index + -- would multiply by the number of indexes instead. + SUM(CASE WHEN p.index_id IN (0, 1) THEN p.row_count ELSE 0 END) AS estimated_rows, + -- Size is table *and* indexes, so every partition counts. Pages are always + -- 8 KB on SQL Server, so the conversion is exact rather than an estimate. + SUM(p.used_page_count) * 8192 AS size_bytes + FROM sys.dm_db_partition_stats p + WHERE p.object_id = t.object_id +) sz LEFT JOIN sys.columns c ON c.object_id = t.object_id WHERE s.name = @p0 AND t.name = @p1 `.trim();