From 4cf9014bd648bd4f1893422e1de4ce214988f4ee Mon Sep 17 00:00:00 2001 From: sumit Date: Sat, 29 Aug 2026 17:18:01 +0530 Subject: [PATCH 1/2] perf/fix: close the remaining Performance-tab gaps from the readiness review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The seven non-blocking findings left over after #86. Each was re-verified against current main before being fixed, and each fix verified after — two of them turned out to be wrong on the first attempt, both caught by testing rather than reading. Sample recovery was an unindexable full scan on a table nothing pruned recoverFullText runs one lookup per sample, up to 20 per "view full query" click, and the query wrapped query_text in three nested regexp_replace/REPLACE calls plus LOWER — so no index could satisfy it and Postgres materialized a rewritten copy of the whole per-connection slice. EXPLAIN (ANALYZE) on a real install: 36 ms at 1,093 rows, 1,112 ms at 34,976. Linear, and query_lineage was absent from SlowQueryRetentionService, so it degraded with the install's *age* rather than its load — which is why it passed every pre-launch test. Precomputed into a STORED generated column (normalized_match) with a text_pattern_ops index, applied by QueryLineageMatchIndexInitializer since this repo has no Flyway runtime. Verified on the real table: Index Scan, 0.119 ms, and the DDL is idempotent on re-run. query_lineage is now purged with the other three fact tables. Deliberately no COALESCE fallback to the inline expression. That is the obvious way to stay safe on a database without the column and it silently undoes the entire fix: measured on the same 37,504-row table, COALESCE(normalized_match, …) plans a Seq Scan at 48.7 ms versus 0.39 ms for the bare column. A missing column instead surfaces as a WARN from the initializer, and recoverFullText already catches a failed lookup and returns the sample unchanged. Denials were retried three times queryClient set retry: 3 with no status predicate, so a 403 became four requests and ~7 s of backoff before the UI could render anything — and an unauthorized /tenant-column-suggestions opens a fresh JDBC connection to the target database on every attempt. The first version of this fix read error.response.status and did nothing at all: the axios response interceptor rethrows a plain Error with the status copied onto error.status, so the axios-shaped field never matched. Measured in the browser before and after: 4 attempts / 7197 ms -> 1 attempt / 34 ms. Confirmed 500, 503, 401 and network failures still retry, and the 3-attempt cap still holds. A customer id containing a slash was unreachable by any encoding customerId is a literal value from the tenant column — application data, so it can contain /, ? or #. Raw, the slash split the path; percent-encoded, Jetty answers 400 "Ambiguous URI path separator". Both reproduced with the real value `acct/77?x=1`, whose rows rendered as "no queries rolled up yet" while the header said the customer had 12 executions. encodeURIComponent alone does not fix this, which is why the id moved off the path: /{connectionId}/customer-queries?customerId=… and /customer-query-samples. The old path routes are kept and @Deprecated for wire compatibility. Verified: the slash-bearing id now returns 200, and 403 on a connection the caller cannot read. Failed fetches rendered as "no data yet" Every panel branched on `!isLoading && rows.length === 0`, and `data ?? []` turns any error into an empty array — so a 404 and an empty result were indistinguishable. That matters more now that connection authorization is enforced: a 403 would read as "nothing captured yet" and send the user to re-run an ingestion they cannot fix. Added a shared QueryError component wired into CustomerExplorer (3 branches), QueryTrendsTab (2) and WorkloadAnalysisPanel (1), with per-status wording verified against the real interceptor error shapes. Tab bar and ARIA At 390 px the four tabs measured 427 px with overflow-x: visible, so three of them sat off-screen unreachable. The bar now scrolls (verified: scrolls 373 px, all four reachable). Completed the ARIA tabs pattern — role="tabpanel", aria-controls, roving tabindex and arrow/Home/End navigation. The first version had a stale-closure bug that moved selection exactly once and then froze; fixed with the functional state updater, verified across the full key sequence including wraparound. Workload reads were gated on the write tier status/latest/getReport/history all used assertCanManageConnectionContent. EffectiveConnectionAccess's own comment lists slow-query analytics under read. Latent today because every grant resolves to FULL_CONTENT, but it would deny the whole Workload tab to a read-only grant the moment one is reintroduced. `run` keeps manage. Known issue, documented rather than fixed The slow-log ingestion cursor has two real defects, both left in place with a comment at updateLastProcessed explaining them: it records the time ingestion *finished* rather than the last event's timestamp (so events arriving mid-run are skipped permanently), and it writes LocalDateTime.now() while every read does .atZone(ZoneOffset.UTC) (agreeing only because the container runs Etc/UTC; a bare-metal install at UTC+5:30 would skip 5.5 h of history every run). Not fixed here because all six providers need live cloud credentials to exercise, and an unverified change to a cursor silently skips or duplicates data. Verification Backend and frontend both build clean. Live against the rebuilt image: new customer routes 200 with a slash-bearing id and 403 on an ungranted connection; workload reads non-403 for a granted user; the lineage index confirmed as an Index Scan on the real table. Not covered: mvn test was not run locally (no JDK/Maven on this host) — CI runs it. Note that the "backend tests (advisory)" job is continue-on-error, so its green tick means the job finished, not that tests passed; main currently has 5 failing test classes independent of this branch. Co-Authored-By: Claude Opus 5 (1M context) --- .../QueryLineageMatchIndexInitializer.java | 116 ++++++++++++++++++ .../SlowQueryAnalyticsController.java | 41 ++++++- .../WorkloadAnalysisController.java | 14 ++- .../repository/QueryLineageRepository.java | 55 +++++++-- .../service/SlowLogIngestionService.java | 24 ++++ .../service/SlowQueryRetentionService.java | 17 ++- .../V118__add_query_lineage_norm_match.sql | 35 ++++++ .../sections/SlowQueriesSection.jsx | 46 ++++++- .../sections/SlowQueriesSection.module.css | 14 +++ .../tabs/Performance/CustomerExplorer.js | 16 ++- .../tabs/Performance/QueryTrendsTab.js | 11 +- .../tabs/Performance/WorkloadAnalysisPanel.js | 12 +- .../tabs/Performance/components/QueryError.js | 47 +++++++ .../tabs/Performance/components/index.js | 1 + src/lib/api/client.js | 30 +++-- src/lib/queryClient.js | 25 +++- 16 files changed, 461 insertions(+), 43 deletions(-) create mode 100644 backend/src/main/java/com/dbaagent/config/QueryLineageMatchIndexInitializer.java create mode 100644 backend/src/main/resources/db/migration/V118__add_query_lineage_norm_match.sql create mode 100644 src/components/tabs/Performance/components/QueryError.js diff --git a/backend/src/main/java/com/dbaagent/config/QueryLineageMatchIndexInitializer.java b/backend/src/main/java/com/dbaagent/config/QueryLineageMatchIndexInitializer.java new file mode 100644 index 0000000..2b8477a --- /dev/null +++ b/backend/src/main/java/com/dbaagent/config/QueryLineageMatchIndexInitializer.java @@ -0,0 +1,116 @@ +package com.dbaagent.config; + +import lombok.extern.slf4j.Slf4j; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.DependsOn; +import org.springframework.jdbc.core.JdbcTemplate; + +import javax.sql.DataSource; + +/** + * Adds the stored normalized column and index that make slow-query sample recovery + * indexable. + * + *

{@code QueryLineageRepository.findLongestByConnectionIdAndNormalizedQueryTextPrefix} + * wraps {@code query_text} in three nested {@code regexp_replace}/{@code REPLACE} calls + * plus {@code LOWER} before comparing it, so no index on {@code query_text} can ever + * satisfy the predicate — Postgres must materialize a rewritten copy of every row in the + * connection's slice. {@code EXPLAIN (ANALYZE)} on a real install: + * + * 

+ *   Seq Scan on query_lineage (actual time=0.244..35.826 rows=422 loops=1)
+ *   Execution Time: 36.179 ms          -- at 1,093 rows
+ *   Execution Time: 1111.996 ms        -- same query, table scaled to 34,976 rows
+ *
+ * + *

{@code SlowQueryAnalyticsService.recoverFullText} runs that once per sample, up to + * 20 per "view full query" click, so one modal open costs ~0.7 s today and ~22 s at 35k + * rows. It degrades with age rather than load, which is why it passes every + * pre-launch test: {@code query_lineage} is not pruned by + * {@code SlowQueryRetentionService} (that only touches {@code slow_query_run}, + * {@code slow_query_customer_day} and {@code slow_query_sample}), so it only grows. + * + *

Precomputing the normalization into a STORED generated column pays the regex chain + * once at write time. Measured on the same scaled table, with the ~120-character prefix + * the caller actually sends: + * + * 

+ *   Index Scan using idx_query_lineage_norm_match
+ *   Execution Time: 0.428 ms           -- vs 1111.996 ms
+ *
+ * + *

The index earns its keep only because the prefix is long and therefore selective. A + * short prefix such as {@code 'select%'} still plans as a sequential scan (~50 ms at 35k + * rows) — that is the precomputation alone, and is fine. Do not "simplify" this by + * dropping the generated column and indexing {@code query_text} directly; the expression, + * not the column, is what the query compares. + * + *

There is no Flyway runtime in this repo (see CLAUDE.md), so this initializer is what + * actually applies {@code V118__add_query_lineage_norm_match.sql}. Both statements are + * {@code IF NOT EXISTS} and the whole thing is best-effort: a failure here costs + * performance, never correctness, since the query returns identical rows either way. + */ +@Configuration +@Slf4j +public class QueryLineageMatchIndexInitializer { + + private static final String TABLE = "query_lineage"; + private static final String COLUMN = "normalized_match"; + private static final String INDEX = "idx_query_lineage_norm_match"; + + /** + * Must match {@code SlowQueryAnalyticsService.normalizeForMatching} exactly, and the + * expression already inlined in the repository query. If one changes, all three do. + */ + private static final String NORMALIZE_EXPR = + "lower(regexp_replace(regexp_replace(" + + "replace(query_text, '`', ''), " + + "'\\s*([.,();])\\s*', '\\1', 'g'), " + + "'\\s+', ' ', 'g'))"; + + @Bean("queryLineageMatchIndexBootstrap") + @DependsOn("entityManagerFactory") + public Object queryLineageMatchIndexBootstrap(DataSource dataSource) { + JdbcTemplate jdbc = new JdbcTemplate(dataSource); + + if (!tableExists(jdbc, TABLE)) { + return new Object(); + } + + try { + jdbc.execute("ALTER TABLE " + TABLE + " ADD COLUMN IF NOT EXISTS " + COLUMN + + " text GENERATED ALWAYS AS (" + NORMALIZE_EXPR + ") STORED"); + } catch (RuntimeException e) { + // Generated columns need Postgres 12+. Older servers keep the sequential scan, + // which is slow but correct, so this must not stop the application. + log.warn("Could not add {}.{} ({}); sample recovery stays on a sequential scan", + TABLE, COLUMN, e.getMessage()); + return new Object(); + } + + try { + // text_pattern_ops so a LIKE 'prefix%' comparison can use the index under any + // collation; the default opclass only helps in the C collation. + jdbc.execute("CREATE INDEX IF NOT EXISTS " + INDEX + " ON " + TABLE + + " (connection_id, " + COLUMN + " text_pattern_ops)"); + } catch (RuntimeException e) { + log.warn("Could not create {}: {}", INDEX, e.getMessage()); + } + + return new Object(); + } + + private static boolean tableExists(JdbcTemplate jdbc, String table) { + try { + Integer found = jdbc.queryForObject( + "SELECT COUNT(*) FROM information_schema.tables " + + "WHERE table_schema = current_schema() AND table_name = ?", + Integer.class, table); + return found != null && found > 0; + } catch (RuntimeException e) { + log.warn("Could not check for table {}: {}", table, e.getMessage()); + return false; + } + } +} diff --git a/backend/src/main/java/com/dbaagent/controller/SlowQueryAnalyticsController.java b/backend/src/main/java/com/dbaagent/controller/SlowQueryAnalyticsController.java index 525d841..91021b8 100644 --- a/backend/src/main/java/com/dbaagent/controller/SlowQueryAnalyticsController.java +++ b/backend/src/main/java/com/dbaagent/controller/SlowQueryAnalyticsController.java @@ -78,7 +78,41 @@ public ResponseEntity> listCusto return ResponseEntity.ok(analyticsService.listCustomers(connectionId)); } - /** Every slow query attributed to one customer, ranked by their mean exec time. */ + /** + * Every slow query attributed to one customer, ranked by their mean exec time. + * + *

The customer id is a query parameter, not a path segment, because it is a + * literal value read out of the tenant column — application data, which can contain + * {@code /}, {@code ?} or {@code #}. Such an id is unreachable as a path segment under + * any encoding: raw, the slash splits the path; percent-encoded, Jetty rejects it with + * {@code 400 Ambiguous URI path separator}. Both were reproduced against this backend + * with the tenant value {@code acct/77?x=1}, whose rows were simply invisible in the + * By-Customer view. + */ + @GetMapping("/{connectionId}/customer-queries") + public ResponseEntity> customerQueries( + @PathVariable String connectionId, + @RequestParam String customerId) { + accessControlService.assertCanReadConnectionContent(connectionId); + return ResponseEntity.ok(analyticsService.queriesForCustomer(connectionId, customerId)); + } + + /** Literal-bearing samples for one (customer, query) pair — copyable SQL. */ + @GetMapping("/{connectionId}/customer-query-samples") + public ResponseEntity> customerQuerySamples( + @PathVariable String connectionId, + @RequestParam String customerId, + @RequestParam String fingerprint) { + accessControlService.assertCanReadConnectionContent(connectionId); + return ResponseEntity.ok( + analyticsService.samplesForCustomerQuery(connectionId, customerId, fingerprint)); + } + + /** + * @deprecated superseded by {@link #customerQueries}; a customer id containing a + * slash cannot be expressed here. Retained so existing clients keep working. + */ + @Deprecated @GetMapping("/{connectionId}/customer/{customerId}/queries") public ResponseEntity> queriesForCustomer( @PathVariable String connectionId, @@ -87,7 +121,10 @@ public ResponseEntity> queriesF return ResponseEntity.ok(analyticsService.queriesForCustomer(connectionId, customerId)); } - /** Literal-bearing samples for one (customer, query) pair — copyable SQL. */ + /** + * @deprecated superseded by {@link #customerQuerySamples}; see above. + */ + @Deprecated @GetMapping("/{connectionId}/customer/{customerId}/query/{fingerprint}/samples") public ResponseEntity> samplesForCustomerQuery( @PathVariable String connectionId, diff --git a/backend/src/main/java/com/dbaagent/controller/WorkloadAnalysisController.java b/backend/src/main/java/com/dbaagent/controller/WorkloadAnalysisController.java index 3f5543b..89acc62 100644 --- a/backend/src/main/java/com/dbaagent/controller/WorkloadAnalysisController.java +++ b/backend/src/main/java/com/dbaagent/controller/WorkloadAnalysisController.java @@ -27,6 +27,12 @@ @Slf4j public class WorkloadAnalysisController { + // Reads use canReadContent; only `run` requires canManageContent. The reads were all + // gated on manage, which is the write tier — EffectiveConnectionAccess's own comment + // lists slow-query analytics under read. Latent today because every grant resolves to + // FULL_CONTENT (so both predicates are true), but it would deny the whole Workload tab + // to a read-only grant the moment one is reintroduced. + private final WorkloadAnalysisService workloadAnalysisService; private final WorkloadAnalysisReportRepository reportRepository; private final AccessControlService accessControlService; @@ -64,7 +70,7 @@ public ResponseEntity> run(@PathVariable String connectionId /** Lightweight poll payload — status + progress without the full report blob. */ @GetMapping("/{connectionId}/status") public ResponseEntity> status(@PathVariable String connectionId) { - accessControlService.assertCanManageConnectionContent(connectionId); + accessControlService.assertCanReadConnectionContent(connectionId); return reportRepository.findFirstByConnectionIdOrderByStartedAtDesc(connectionId) .map(r -> ResponseEntity.ok(Map.of( "reportId", r.getId(), @@ -81,7 +87,7 @@ public ResponseEntity> status(@PathVariable String connectio /** The full latest report (with the composed sections). 204 if none yet. */ @GetMapping("/{connectionId}/latest") public ResponseEntity latest(@PathVariable String connectionId) { - accessControlService.assertCanManageConnectionContent(connectionId); + accessControlService.assertCanReadConnectionContent(connectionId); return reportRepository.findFirstByConnectionIdOrderByStartedAtDesc(connectionId) .map(ResponseEntity::ok) .orElseGet(() -> ResponseEntity.noContent().build()); @@ -92,7 +98,7 @@ public ResponseEntity latest(@PathVariable String connec public ResponseEntity getReport(@PathVariable String reportId) { return reportRepository.findById(reportId) .map(r -> { - accessControlService.assertCanManageConnectionContent(r.getConnectionId()); + accessControlService.assertCanReadConnectionContent(r.getConnectionId()); return ResponseEntity.ok(r); }) .orElseGet(() -> ResponseEntity.notFound().build()); @@ -101,7 +107,7 @@ public ResponseEntity getReport(@PathVariable String rep /** Recent report history (newest first), metadata only via the entity. */ @GetMapping("/{connectionId}/history") public ResponseEntity> history(@PathVariable String connectionId) { - accessControlService.assertCanManageConnectionContent(connectionId); + accessControlService.assertCanReadConnectionContent(connectionId); return ResponseEntity.ok(reportRepository.findTop20ByConnectionIdOrderByStartedAtDesc(connectionId)); } } diff --git a/backend/src/main/java/com/dbaagent/repository/QueryLineageRepository.java b/backend/src/main/java/com/dbaagent/repository/QueryLineageRepository.java index 1a3d107..0ef211e 100644 --- a/backend/src/main/java/com/dbaagent/repository/QueryLineageRepository.java +++ b/backend/src/main/java/com/dbaagent/repository/QueryLineageRepository.java @@ -3,8 +3,10 @@ import com.dbaagent.model.QueryLineage; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Query; +import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.domain.Pageable; import org.springframework.data.repository.query.Param; +import org.springframework.transaction.annotation.Transactional; import org.springframework.stereotype.Repository; import java.time.LocalDateTime; @@ -147,23 +149,32 @@ QueryLineage findLongestByConnectionIdAndCollapsedQueryTextPrefix( * Callers must apply the SAME transformation to their prefix on the * Java side (see {@code SlowQueryAnalyticsService.normalizeForMatching}). * - *

The regex chain runs once per candidate row, so this is intended for - * single-row lookups (LIMIT 1) over the per-connection slice. For - * d840f866-style connections (~5K lineage rows) it returns in a few ms; - * larger connections may benefit from a functional index on the - * normalized expression, but none is needed yet. + *

The normalization is precomputed into the stored generated column + * {@code normalized_match} ({@code QueryLineageMatchIndexInitializer}) and matched + * against that, so the regex chain runs once at write time rather than once per + * candidate row on every read. Inlining the expression here made the predicate + * unindexable: Postgres had to materialize a rewritten copy of the whole + * per-connection slice, which measured 36 ms at 1,093 rows and 1,112 ms at 34,976 — + * and {@code recoverFullText} issues this up to 20 times per "view full query" click, + * against a table no retention job prunes. On the same scaled table with the ~120-char + * prefix the caller actually sends, the indexed form plans as an Index Scan at + * 0.428 ms. + * + *

Matched against the column directly, with no {@code COALESCE} fallback to + * the inline expression. That fallback is the obvious way to stay safe on a database + * without the column, and it silently undoes the whole fix: wrapping the column in + * {@code COALESCE(...)} makes the predicate non-indexable again. Measured on the same + * 37,504-row table — {@code COALESCE(normalized_match, …)} plans a Seq Scan at + * 48.7 ms, the bare column an Index Scan at 0.39 ms. If the column is ever absent, + * {@code QueryLineageMatchIndexInitializer} logs it at WARN and + * {@code SlowQueryAnalyticsService.recoverFullText} already treats a failed lookup as + * "no longer text available" and returns the sample unchanged. */ @Query(value = """ SELECT * FROM query_lineage WHERE connection_id = :connectionId AND LENGTH(query_text) > :minLength - AND LOWER( - regexp_replace( - regexp_replace( - REPLACE(query_text, '`', ''), - '\\s*([.,();])\\s*', '\\1', 'g'), - '\\s+', ' ', 'g') - ) LIKE :escapedPrefix ESCAPE '\\' + AND normalized_match LIKE :escapedPrefix ESCAPE '\\' ORDER BY LENGTH(query_text) DESC LIMIT 1 """, nativeQuery = true) @@ -172,4 +183,24 @@ QueryLineage findLongestByConnectionIdAndNormalizedQueryTextPrefix( @Param("escapedPrefix") String escapedPrefix, @Param("minLength") int minLength ); + + /** + * Drop lineage rows older than the connection's retention window. + * + *

This table was never pruned: {@code SlowQueryRetentionService} covered + * {@code slow_query_run}, {@code slow_query_customer_day} and + * {@code slow_query_sample} but not lineage, so it grew without bound while the + * 30-day analytics tables stayed small. That is what made sample recovery degrade + * with age rather than load — the scanned table kept growing even on an idle install. + * + *

Keyed on {@code created_at}, which is non-null and already indexed + * ({@code idx_query_lineage_created}). + */ + @Modifying + @Transactional + @Query("DELETE FROM QueryLineage q WHERE q.connectionId = :connectionId " + + "AND q.createdAt < :cutoff") + int deleteByConnectionIdAndCreatedAtBefore( + @Param("connectionId") String connectionId, + @Param("cutoff") java.time.LocalDateTime cutoff); } diff --git a/backend/src/main/java/com/dbaagent/service/SlowLogIngestionService.java b/backend/src/main/java/com/dbaagent/service/SlowLogIngestionService.java index c3509b3..3226966 100644 --- a/backend/src/main/java/com/dbaagent/service/SlowLogIngestionService.java +++ b/backend/src/main/java/com/dbaagent/service/SlowLogIngestionService.java @@ -418,6 +418,30 @@ private boolean isFrequencySatisfied(SlowLogSourceConfig config) { return last.plusMinutes(freq).isBefore(LocalDateTime.now()); } + /** + * KNOWN ISSUE — this cursor has two defects, both unfixed. Deliberately left alone + * rather than changed blind: every provider needs live cloud credentials to exercise, + * so a fix cannot be verified end to end here, and getting it wrong silently skips or + * duplicates slow-query events. + * + *

    + *
  1. Events arriving mid-run are lost. The cursor is set to the time + * ingestion finished, but {@code SINCE_LAST} then uses it as an + * exclusive lower bound (see {@code resolveStartTime}). Anything whose timestamp + * falls between the last parsed event and this write is never fetched — a gap + * proportional to how long the run took, so worst on the slowest S3/CloudWatch + * pulls. The fix is to record the maximum event timestamp actually parsed. + *
  2. Local time written, UTC read. {@code LocalDateTime.now()} is + * server-local; every read does {@code .atZone(ZoneOffset.UTC)}. The Compose + * image runs {@code Etc/UTC} so the two agree there by luck, but on a bare-metal + * install in, say, UTC+5:30, the cursor lands 5.5 h in the future and that much + * slow-query history is skipped on every run (west of UTC it goes backwards and + * re-ingests duplicates). The fix is to store an {@code Instant}/ + * {@code timestamptz} end to end. + *
+ * + *

{@code SlowLogSourceConfigService.updateAfterAutoIngestion} has the same bug. + */ private void updateLastProcessed(SlowLogSourceConfig config) { config.setLastProcessedAt(LocalDateTime.now()); config.setUpdatedAt(LocalDateTime.now()); diff --git a/backend/src/main/java/com/dbaagent/service/SlowQueryRetentionService.java b/backend/src/main/java/com/dbaagent/service/SlowQueryRetentionService.java index 442285c..9134c21 100644 --- a/backend/src/main/java/com/dbaagent/service/SlowQueryRetentionService.java +++ b/backend/src/main/java/com/dbaagent/service/SlowQueryRetentionService.java @@ -4,6 +4,7 @@ import com.dbaagent.repository.ResourceLimitsRepository; import com.dbaagent.repository.SlowQueryCustomerDayRepository; import com.dbaagent.repository.SlowQueryRunRepository; +import com.dbaagent.repository.QueryLineageRepository; import com.dbaagent.repository.SlowQuerySampleRepository; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -32,6 +33,7 @@ public class SlowQueryRetentionService { private final SlowQueryRunRepository runRepository; private final SlowQuerySampleRepository sampleRepository; private final SlowQueryCustomerDayRepository customerDayRepository; + private final QueryLineageRepository queryLineageRepository; private final ResourceLimitsRepository resourceLimitsRepository; /** Resolve the retention window for a connection (days). */ @@ -44,7 +46,12 @@ public int retentionDaysFor(String connectionId) { /** * Delete slow-query analytics rows older than the connection's retention - * window across all three fact tables. Safe to call repeatedly. + * window across all four fact tables. Safe to call repeatedly. + * + *

{@code query_lineage} was missing here, and that mattered more than the row + * count suggests: it is the table {@code recoverFullText} scans up to 20 times per + * "view full query" click, so leaving it unpruned made sample recovery degrade with + * the install's age while the other three tables stayed bounded at 30 days. */ @Transactional public void purge(String connectionId) { @@ -55,11 +62,13 @@ public void purge(String connectionId) { int customerDays = customerDayRepository.deleteByConnectionIdAndDayBefore(connectionId, cutoff); int samples = sampleRepository.deleteByConnectionIdAndCapturedAtBefore( connectionId, cutoff.atStartOfDay()); + int lineage = queryLineageRepository.deleteByConnectionIdAndCreatedAtBefore( + connectionId, cutoff.atStartOfDay()); - if (runs > 0 || customerDays > 0 || samples > 0) { + if (runs > 0 || customerDays > 0 || samples > 0 || lineage > 0) { log.info("Slow-query retention purge for connection {} (>{}d): " - + "{} runs, {} customer-days, {} samples removed", - connectionId, days, runs, customerDays, samples); + + "{} runs, {} customer-days, {} samples, {} lineage rows removed", + connectionId, days, runs, customerDays, samples, lineage); } } } diff --git a/backend/src/main/resources/db/migration/V118__add_query_lineage_norm_match.sql b/backend/src/main/resources/db/migration/V118__add_query_lineage_norm_match.sql new file mode 100644 index 0000000..ebd5203 --- /dev/null +++ b/backend/src/main/resources/db/migration/V118__add_query_lineage_norm_match.sql @@ -0,0 +1,35 @@ +-- Make slow-query sample recovery indexable. +-- +-- QueryLineageRepository.findLongestByConnectionIdAndNormalizedQueryTextPrefix used to +-- wrap query_text in three nested regexp_replace/REPLACE calls plus LOWER before +-- comparing, so no index could satisfy the predicate. Postgres materialized a rewritten +-- copy of every row in the connection's slice, and SlowQueryAnalyticsService +-- .recoverFullText issues that up to 20 times per "view full query" click. +-- +-- Measured with EXPLAIN (ANALYZE) on a real install: +-- 1,093 rows -> 36 ms per call (~0.7 s per modal open) +-- 34,976 rows -> 1112 ms per call (~22 s per modal open) +-- The growth is linear, and query_lineage was not pruned by SlowQueryRetentionService, +-- so this degraded with the install's age rather than its load — which is why it passed +-- every pre-launch test. +-- +-- Precomputing the normalization into a STORED generated column pays the regex chain once +-- at write time. On the same 34,976-row table, with the ~120-character prefix the caller +-- actually sends: Index Scan, 0.428 ms. +-- +-- NOTE: this repo has no Flyway runtime (see CLAUDE.md). QueryLineageMatchIndexInitializer +-- is what actually applies these statements at startup; this file is the changelog record. + +ALTER TABLE query_lineage + ADD COLUMN IF NOT EXISTS normalized_match text + GENERATED ALWAYS AS ( + lower(regexp_replace(regexp_replace( + replace(query_text, '`', ''), + '\s*([.,();])\s*', '\1', 'g'), + '\s+', ' ', 'g')) + ) STORED; + +-- text_pattern_ops so LIKE 'prefix%' can use the index under any collation; the default +-- opclass only helps in the C collation. +CREATE INDEX IF NOT EXISTS idx_query_lineage_norm_match + ON query_lineage (connection_id, normalized_match text_pattern_ops); diff --git a/src/components/sections/SlowQueriesSection.jsx b/src/components/sections/SlowQueriesSection.jsx index eb320dd..44af4c0 100644 --- a/src/components/sections/SlowQueriesSection.jsx +++ b/src/components/sections/SlowQueriesSection.jsx @@ -1,4 +1,4 @@ -import { useState } from 'react' +import { useRef, useState } from 'react' import { Activity, FileText, LineChart, Settings, Users } from 'lucide-react' import { useConnectionManager } from '@/lib/hooks/useConnectionManager' import { useSlowLogSourceConfig } from '@/lib/hooks/queries' @@ -33,6 +33,28 @@ const LOG_SOURCE_HELP = { export default function SlowQueriesSection() { const { connectionId, selectedConnection } = useConnectionManager() const [tab, setTab] = useState('trends') + const tabRefs = useRef({}) + + /** Arrow / Home / End move between tabs, as the ARIA tabs pattern expects. */ + const onTabKeyDown = (e) => { + const step = { ArrowRight: 1, ArrowLeft: -1 }[e.key] + if (step === undefined && e.key !== 'Home' && e.key !== 'End') return + e.preventDefault() + // Resolve the next tab from the *current* state, not the `tab` captured when this + // handler was created: two keypresses within one render would otherwise both move + // relative to the same starting index and selection would stick after the first. + setTab((current) => { + const i = TABS.findIndex((t) => t.id === current) + const next = e.key === 'Home' ? 0 + : e.key === 'End' ? TABS.length - 1 + : (i + step + TABS.length) % TABS.length + const id = TABS[next].id + // Focus follows selection, per the ARIA tabs pattern. Deferred so the tab is + // already rendered with tabIndex=0 when we focus it. + queueMicrotask(() => tabRefs.current[id]?.focus()) + return id + }) + } const [logSourceModalOpen, setLogSourceModalOpen] = useState(false) const logSourceQ = useSlowLogSourceConfig(connectionId) const hasLogSource = Boolean(logSourceQ.data?.id) @@ -86,16 +108,28 @@ export default function SlowQueriesSection() { ) : ( <>

-
+
{TABS.map((t) => { const Icon = t.icon const active = tab === t.id return (
-
+
{tab === 'trends' && } {tab === 'customers' && } {tab === 'workload' && } diff --git a/src/components/sections/SlowQueriesSection.module.css b/src/components/sections/SlowQueriesSection.module.css index 1a7172c..3b356ad 100644 --- a/src/components/sections/SlowQueriesSection.module.css +++ b/src/components/sections/SlowQueriesSection.module.css @@ -6,6 +6,17 @@ border-radius: 14px; background: #f9fafb; width: fit-content; + /* At 390px the four tabs measure 427px, so three of them sat off-screen with no way + to reach them: `width: fit-content` lets the bar grow past the viewport and the + parent clipped it. Scroll the bar itself rather than wrapping, so the pill keeps + its shape; max-width caps it at the container so the rule has something to act on. */ + max-width: 100%; + overflow-x: auto; + scrollbar-width: none; +} + +.tabBar::-webkit-scrollbar { + display: none; } .toolbar { @@ -18,6 +29,9 @@ .tabButton { display: inline-flex; + /* Do not shrink inside the scrolling .tabBar — otherwise labels compress to + illegible slivers instead of the bar scrolling. */ + flex: 0 0 auto; align-items: center; gap: 8px; padding: 8px 14px; diff --git a/src/components/tabs/Performance/CustomerExplorer.js b/src/components/tabs/Performance/CustomerExplorer.js index e547e82..7009d54 100644 --- a/src/components/tabs/Performance/CustomerExplorer.js +++ b/src/components/tabs/Performance/CustomerExplorer.js @@ -13,6 +13,7 @@ import { useState } from "react"; import { useQuery } from "@tanstack/react-query"; import { Clock, Users, Copy, Check, X } from "lucide-react"; import { slowQueryAnalyticsAPI } from "@/lib/api/client"; +import { QueryError } from "./components"; import styles from "./CustomerExplorer.module.css"; const fmtDuration = (v) => { @@ -71,7 +72,10 @@ function QuerySamplesModal({ connectionId, customerId, customerName, fingerprint
{samplesQ.isLoading &&
Loading samples…
} - {!samplesQ.isLoading && samples.length === 0 && ( + {samplesQ.isError && ( + + )} + {!samplesQ.isLoading && !samplesQ.isError && samples.length === 0 && (
No literal-bearing samples captured for this customer + query yet. Slow-log ingestion gives the fullest coverage. @@ -130,7 +134,10 @@ export default function CustomerExplorer({ connectionId }) { Customers {customers.length > 0 && ({customers.length})}
{customersQ.isLoading &&
Loading…
} - {!customersQ.isLoading && customers.length === 0 && ( + {customersQ.isError && ( + + )} + {!customersQ.isLoading && !customersQ.isError && customers.length === 0 && (
No customer-attributed slow queries yet. Configure the tenant column under Settings, then run an analysis. @@ -193,7 +200,10 @@ export default function CustomerExplorer({ connectionId }) { {queriesQ.isLoading && (
Loading queries…
)} - {!queriesQ.isLoading && queries.length === 0 && ( + {queriesQ.isError && ( + + )} + {!queriesQ.isLoading && !queriesQ.isError && queries.length === 0 && (
No queries rolled up yet for this customer.
diff --git a/src/components/tabs/Performance/QueryTrendsTab.js b/src/components/tabs/Performance/QueryTrendsTab.js index b256c59..077f264 100644 --- a/src/components/tabs/Performance/QueryTrendsTab.js +++ b/src/components/tabs/Performance/QueryTrendsTab.js @@ -21,6 +21,7 @@ import { RefreshCw, TrendingUp, AlertTriangle, Clock, Users, Copy, Check, FileTe import { slowQueryAnalyticsAPI } from "@/lib/api/client"; import { useConnectionManager } from "@/lib/hooks/useConnectionManager"; import SlowQuerySourceModal from "@/components/SlowQuerySourceModal"; +import { QueryError } from "./components"; import styles from "./QueryTrendsTab.module.css"; /** Human-readable duration: ms under a second, seconds under a minute, else minutes. */ @@ -356,7 +357,10 @@ export default function QueryTrendsTab({ connectionId }) {
Mean execution time / day
{timelineQ.isLoading &&
Loading timeline…
} - {!timelineQ.isLoading && timeline.length === 0 && ( + {timelineQ.isError && ( + + )} + {!timelineQ.isLoading && !timelineQ.isError && timeline.length === 0 && (
No timeline data yet for this query.
)} {timeline.length > 0 && ( @@ -405,7 +409,10 @@ export default function QueryTrendsTab({ connectionId }) { Per-customer breakdown
{customersQ.isLoading &&
Loading…
} - {!customersQ.isLoading && customers.length === 0 && ( + {customersQ.isError && ( + + )} + {!customersQ.isLoading && !customersQ.isError && customers.length === 0 && (
No per-customer data for this query — it isn’t filtered by the tenant column, or no literal-bearing sample was captured. diff --git a/src/components/tabs/Performance/WorkloadAnalysisPanel.js b/src/components/tabs/Performance/WorkloadAnalysisPanel.js index 16f500d..31c799b 100644 --- a/src/components/tabs/Performance/WorkloadAnalysisPanel.js +++ b/src/components/tabs/Performance/WorkloadAnalysisPanel.js @@ -28,6 +28,7 @@ import { } from "lucide-react"; import { workloadAnalysisAPI } from "@/lib/api/client"; import { HelpTooltip } from "@/components/tabs/Brain/components/HelpTooltip"; +import { QueryError } from "./components"; import styles from "./WorkloadAnalysisPanel.module.css"; const fmtMs = (v) => { @@ -194,8 +195,17 @@ export default function WorkloadAnalysisPanel({ connectionId }) {
)} + {/* load failure — distinct from "never run", which looks identical otherwise */} + {latestQ.isError && ( + + )} + {/* empty state — never run */} - {!latest && !latestQ.isLoading && ( + {!latest && !latestQ.isLoading && !latestQ.isError && (

No workload analysis yet

diff --git a/src/components/tabs/Performance/components/QueryError.js b/src/components/tabs/Performance/components/QueryError.js new file mode 100644 index 0000000..b2878fc --- /dev/null +++ b/src/components/tabs/Performance/components/QueryError.js @@ -0,0 +1,47 @@ +import { AlertTriangle } from "lucide-react"; + +/** + * The error state for an async panel in the Performance tab. + * + *

Exists because every panel here rendered a failed fetch and an empty result + * identically: the branch was `!isLoading && rows.length === 0`, and `data ?? []` turns + * any error into an empty array. Clicking a customer whose row said "1 slow query · 12 + * executions" produced "No queries rolled up yet for this customer" when the request had + * actually 404'd — the data existed and the UI reported absence. + * + *

That matters most for 403s: with connection authorization now enforced, a user + * without access to a connection gets a denial, and rendering it as "nothing captured + * yet" would send them to re-run an ingestion they cannot fix. + */ +export function QueryError({ error, what = "data", className }) { + // `error.status`, not `error.response.status`: the axios response interceptor in + // api/client.js rethrows a plain Error with the status copied onto it. Reading the + // axios-shaped field silently never matches, so every denial fell through to the + // generic branch. `responseData` is where the interceptor puts the parsed body. + const status = error?.status ?? error?.response?.status; + + let message; + if (status === 403) { + message = `You don't have access to this connection's ${what}.`; + } else if (status === 404) { + message = `Not found — the ${what} may have been removed, or belong to another connection.`; + } else if (status === 412) { + message = `Not configured for this connection yet.`; + } else { + // Prefer the server's own wording; the axios interceptor puts it on `message`. + message = error?.responseData?.message + || error?.response?.data?.message + || error?.message + || `Could not load ${what}.`; + } + + return ( +

+
+ ); +} + +export default QueryError; diff --git a/src/components/tabs/Performance/components/index.js b/src/components/tabs/Performance/components/index.js index f16e9db..58ed632 100644 --- a/src/components/tabs/Performance/components/index.js +++ b/src/components/tabs/Performance/components/index.js @@ -4,3 +4,4 @@ export { default as TableHeatmap } from './TableHeatmap' export { default as InsightsList } from './InsightsList' export { default as QueryDetailDialog } from './QueryDetailDialog' export { default as PerformanceActionCard } from './PerformanceActionCard' +export { default as QueryError } from './QueryError' diff --git a/src/lib/api/client.js b/src/lib/api/client.js index d98c513..b859c9f 100644 --- a/src/lib/api/client.js +++ b/src/lib/api/client.js @@ -3878,54 +3878,62 @@ export const slackDigestAPI = { export const slowQueryAnalyticsAPI = { getQueries: (connectionId) => apiClient - .get(`/api/slow-query-analytics/${connectionId}/queries`) + .get(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/queries`) .then((r) => r.data), getTimeline: (connectionId, fingerprint) => apiClient - .get(`/api/slow-query-analytics/${connectionId}/timeline/${fingerprint}`) + .get(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/timeline/${encodeURIComponent(fingerprint)}`) .then((r) => r.data), getRegressions: (connectionId, minFactor = 1.5) => apiClient - .get(`/api/slow-query-analytics/${connectionId}/regressions`, { + .get(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/regressions`, { params: { minFactor }, }) .then((r) => r.data), getCustomers: (connectionId, fingerprint, day = null) => apiClient .get( - `/api/slow-query-analytics/${connectionId}/query/${fingerprint}/customers`, + `/api/slow-query-analytics/${encodeURIComponent(connectionId)}/query/${encodeURIComponent(fingerprint)}/customers`, { params: day ? { day } : {} }, ) .then((r) => r.data), getSamples: (connectionId, fingerprint) => apiClient - .get(`/api/slow-query-analytics/${connectionId}/query/${fingerprint}/samples`) + .get(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/query/${encodeURIComponent(fingerprint)}/samples`) .then((r) => r.data), listCustomers: (connectionId) => apiClient - .get(`/api/slow-query-analytics/${connectionId}/customers`) + .get(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/customers`) .then((r) => r.data), + // customerId is a literal value from the tenant column, so it is application data and + // may contain "/", "?" or "#". It cannot go in a path segment: raw, the slash splits + // the path; percent-encoded, Jetty answers 400 "Ambiguous URI path separator". Both + // were reproduced with the real tenant value `acct/77?x=1`, whose rows silently showed + // as "no queries rolled up yet". axios encodes `params` for us. getCustomerQueries: (connectionId, customerId) => apiClient - .get(`/api/slow-query-analytics/${connectionId}/customer/${customerId}/queries`) + .get(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/customer-queries`, { + params: { customerId }, + }) .then((r) => r.data), getCustomerQuerySamples: (connectionId, customerId, fingerprint) => apiClient .get( - `/api/slow-query-analytics/${connectionId}/customer/${customerId}/query/${fingerprint}/samples`, + `/api/slow-query-analytics/${encodeURIComponent(connectionId)}/customer-query-samples`, + { params: { customerId, fingerprint } }, ) .then((r) => r.data), getTenantColumnSuggestions: (connectionId) => apiClient - .get(`/api/slow-query-analytics/${connectionId}/tenant-column-suggestions`) + .get(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/tenant-column-suggestions`) .then((r) => r.data), getConfig: (connectionId) => apiClient - .get(`/api/slow-query-analytics/${connectionId}/config`) + .get(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/config`) .then((r) => r.data), putConfig: (connectionId, body) => apiClient - .put(`/api/slow-query-analytics/${connectionId}/config`, body) + .put(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/config`, body) .then((r) => r.data), analyzeNow: (connectionId) => apiClient diff --git a/src/lib/queryClient.js b/src/lib/queryClient.js index a08582e..9506071 100644 --- a/src/lib/queryClient.js +++ b/src/lib/queryClient.js @@ -1,5 +1,12 @@ import { QueryClient } from '@tanstack/react-query' +/** + * Statuses that will never succeed on retry: the request was understood and refused. + * 401 is excluded on purpose — the axios interceptor refreshes the token and a retry + * genuinely can succeed after it. + */ +const NON_RETRYABLE_STATUSES = new Set([400, 403, 404, 405, 409, 412, 422]) + /** * React Query client configuration * Used for caching and retry logic across the app @@ -9,7 +16,23 @@ export const queryClient = new QueryClient({ queries: { staleTime: 5 * 60 * 1000, // Data is fresh for 5 minutes gcTime: 10 * 60 * 1000, // Garbage collect after 10 minutes (v5: renamed from cacheTime) - retry: 3, // Retry failed requests 3 times + // Retry transient failures only. A 403/404 is a settled answer, and retrying + // it costs three extra round trips plus ~7s of backoff (1s+2s+4s) before the + // UI can show anything — observed as four identical requests in the console + // for one denied call. Some of these are expensive on the server too: an + // unauthorized /tenant-column-suggestions opens a fresh JDBC connection to + // the target database on every attempt. + retry: (failureCount, error) => { + // `error.status`, not `error.response.status`: the axios response + // interceptor in api/client.js rethrows a plain Error with the status + // copied onto it, so `response` is gone by the time react-query sees it. + // Reading the axios-shaped field looks right and silently never matches — + // the denied request still made four attempts over ~7s. `response.status` + // is kept as a fallback for any caller that bypasses the interceptor. + const status = error?.status ?? error?.response?.status + if (NON_RETRYABLE_STATUSES.has(status)) return false + return failureCount < 3 + }, retryDelay: (attemptIndex) => Math.min(1000 * 2 ** attemptIndex, 30000), // Exponential backoff refetchOnWindowFocus: false, // Don't refetch on window focus refetchOnReconnect: true, // Refetch on reconnect From 06476bf9e748641cac4f88b209e91ba16c4112a5 Mon Sep 17 00:00:00 2001 From: sumit Date: Sat, 29 Aug 2026 17:45:20 +0530 Subject: [PATCH 2/2] fix: trim the generated normalized_match to match Java's normalizeForMatching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found in hands-on QA of the previous commit, not by reading it. SlowQueryAnalyticsService.normalizeForMatching ends with .trim(); the SQL expression behind the generated column did not. A lineage row stored with leading whitespace therefore normalized to " select ..." in the column and "select ..." on the Java side, so the prefix LIKE never matched and recoverFullText silently returned the truncated sample instead of the full SQL. Verified against the local install: inserting ' SELECT x FROM t WHERE y = 1 ' produced "[ select x from t where y = 1 ]" where Java produces "[select x from t where y = 1]"; 4 of 1,174 real rows carried such whitespace. The flaw was equally present in the inline expression this column replaced, so it is pre-existing rather than a regression — but it is silent either way, which is why it survived. Two parts to the fix: * btrim(...) added to the expression in both the initializer and V118. * The initializer now detects a stale column and rebuilds it. A generated column's expression cannot be altered in place and ADD COLUMN IF NOT EXISTS silently keeps whatever is already there, so an install that ran the earlier build would have kept the untrimmed expression forever. It compares pg_get_expr against the expected shape and only drops/re-adds when they differ, so a normal restart does not rewrite the table. Verified on the running stack: restart logged "Rebuilding query_lineage.normalized_match: stored expression is out of date", the stored expression now carries btrim, the index survived the rebuild, and rows-with-untrimmed-normalization went from 4 to 0. Co-Authored-By: Claude Opus 5 (1M context) --- .../QueryLineageMatchIndexInitializer.java | 58 +++++++++++++++++-- .../V118__add_query_lineage_norm_match.sql | 9 ++- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/backend/src/main/java/com/dbaagent/config/QueryLineageMatchIndexInitializer.java b/backend/src/main/java/com/dbaagent/config/QueryLineageMatchIndexInitializer.java index 2b8477a..7d986a2 100644 --- a/backend/src/main/java/com/dbaagent/config/QueryLineageMatchIndexInitializer.java +++ b/backend/src/main/java/com/dbaagent/config/QueryLineageMatchIndexInitializer.java @@ -60,14 +60,21 @@ public class QueryLineageMatchIndexInitializer { private static final String INDEX = "idx_query_lineage_norm_match"; /** - * Must match {@code SlowQueryAnalyticsService.normalizeForMatching} exactly, and the - * expression already inlined in the repository query. If one changes, all three do. + * Must match {@code SlowQueryAnalyticsService.normalizeForMatching} exactly. If one + * changes, both do. + * + *

The {@code btrim} is load-bearing and was missing from the expression this + * replaces: Java's {@code normalizeForMatching} ends with {@code .trim()}, so a + * lineage row stored with leading whitespace normalized to {@code " select ..."} on + * the SQL side and {@code "select ..."} on the Java side. The prefix {@code LIKE} + * then never matched and recovery silently returned the truncated sample. 4 of 1,174 + * rows on the local install carry such whitespace. */ private static final String NORMALIZE_EXPR = - "lower(regexp_replace(regexp_replace(" + "btrim(lower(regexp_replace(regexp_replace(" + "replace(query_text, '`', ''), " + "'\\s*([.,();])\\s*', '\\1', 'g'), " - + "'\\s+', ' ', 'g'))"; + + "'\\s+', ' ', 'g')))"; @Bean("queryLineageMatchIndexBootstrap") @DependsOn("entityManagerFactory") @@ -78,6 +85,20 @@ public Object queryLineageMatchIndexBootstrap(DataSource dataSource) { return new Object(); } + // A generated column's expression cannot be altered in place, and ADD COLUMN IF + // NOT EXISTS silently keeps whatever definition is already there. An install that + // ran an earlier build of this initializer therefore keeps the untrimmed + // expression forever unless the column is dropped first. Only drop when the + // definition actually differs, so a normal restart does not rewrite the table. + if (columnDefinitionDiffers(jdbc)) { + log.info("Rebuilding {}.{}: stored expression is out of date", TABLE, COLUMN); + try { + jdbc.execute("ALTER TABLE " + TABLE + " DROP COLUMN " + COLUMN); + } catch (RuntimeException e) { + log.warn("Could not drop stale {}.{}: {}", TABLE, COLUMN, e.getMessage()); + } + } + try { jdbc.execute("ALTER TABLE " + TABLE + " ADD COLUMN IF NOT EXISTS " + COLUMN + " text GENERATED ALWAYS AS (" + NORMALIZE_EXPR + ") STORED"); @@ -101,6 +122,35 @@ public Object queryLineageMatchIndexBootstrap(DataSource dataSource) { return new Object(); } + /** + * True when {@code normalized_match} exists but was generated by a different + * expression than {@link #NORMALIZE_EXPR}. Compared on the normalized form Postgres + * stores in {@code pg_get_expr}, with whitespace collapsed, since the server rewrites + * the text it was given (adds casts, reorders parens) and a literal comparison would + * report a difference on every start and rewrite the table each time. + */ + private static boolean columnDefinitionDiffers(JdbcTemplate jdbc) { + try { + String stored = jdbc.query( + "SELECT pg_get_expr(d.adbin, d.adrelid) " + + "FROM pg_attrdef d " + + "JOIN pg_attribute a ON a.attrelid = d.adrelid AND a.attnum = d.adnum " + + "WHERE d.adrelid = ?::regclass AND a.attname = ?", + rs -> rs.next() ? rs.getString(1) : null, TABLE, COLUMN); + if (stored == null) { + return false; // column not present yet — nothing stale to drop + } + return !squash(stored).contains("btrim"); + } catch (RuntimeException e) { + log.warn("Could not inspect {}.{} definition: {}", TABLE, COLUMN, e.getMessage()); + return false; + } + } + + private static String squash(String s) { + return s.replaceAll("\\s+", "").toLowerCase(); + } + private static boolean tableExists(JdbcTemplate jdbc, String table) { try { Integer found = jdbc.queryForObject( diff --git a/backend/src/main/resources/db/migration/V118__add_query_lineage_norm_match.sql b/backend/src/main/resources/db/migration/V118__add_query_lineage_norm_match.sql index ebd5203..26da180 100644 --- a/backend/src/main/resources/db/migration/V118__add_query_lineage_norm_match.sql +++ b/backend/src/main/resources/db/migration/V118__add_query_lineage_norm_match.sql @@ -17,16 +17,21 @@ -- at write time. On the same 34,976-row table, with the ~120-character prefix the caller -- actually sends: Index Scan, 0.428 ms. -- +-- The btrim matches Java's normalizeForMatching, which ends with .trim(). Without it a +-- row stored with leading whitespace normalizes to " select ..." here but "select ..." in +-- Java, so the prefix LIKE never matches and recovery silently returns the truncated +-- sample. That flaw was present in the inline expression this replaces. +-- -- NOTE: this repo has no Flyway runtime (see CLAUDE.md). QueryLineageMatchIndexInitializer -- is what actually applies these statements at startup; this file is the changelog record. ALTER TABLE query_lineage ADD COLUMN IF NOT EXISTS normalized_match text GENERATED ALWAYS AS ( - lower(regexp_replace(regexp_replace( + btrim(lower(regexp_replace(regexp_replace( replace(query_text, '`', ''), '\s*([.,();])\s*', '\1', 'g'), - '\s+', ' ', 'g')) + '\s+', ' ', 'g'))) ) STORED; -- text_pattern_ops so LIKE 'prefix%' can use the index under any collation; the default