experiment(ui): diff UI POC#183
Conversation
| query_b_id: string; | ||
| } | ||
|
|
||
| export type QueryProfileDiffScenario = 'plans_equal' | 'plans_different' | 'plans_incomparable'; |
There was a problem hiding this comment.
We'll need to define what makes two plans "incomparable" vs "different" (e.g. different SQL text? different engines?)
There was a problem hiding this comment.
Yeah this will definitely need some figuring, It's also likely we'll need this flag on various things (eg. query plan, resource tree), so should probably make this a more generic "can these things be compared" flag or something
|
|
||
| export type QueryProfileDiffScenario = 'plans_equal' | 'plans_different' | 'plans_incomparable'; | ||
|
|
||
| export interface QueryProfileDiffQuerySummary { |
There was a problem hiding this comment.
add:
duration_s: number; // rather than deriving it from the query bundle
| plan_id: string | null; | ||
| } | ||
|
|
||
| export interface QueryProfileDiffStatDelta { |
There was a problem hiding this comment.
We could consider adding a FE-only derived type that wraps this with a direction field ('better' | 'worse' | 'neutral'). The direction is already being re-derived from the delta in multiple places (QueryDiffTable.utils, QueryDiffStats.utils)
There was a problem hiding this comment.
This could add some semantic value... i'm thinking if the analyzer can infer something that the FE can't (maybe a higher value is better for specific stats?) then this could be very useful.
|
|
||
| export interface QueryProfileDiffPlanComparison { | ||
| /* Big question here, how do we represent query plan graph diffs */ | ||
| match_kind: 'structural' | 'different' | 'incomparable'; |
There was a problem hiding this comment.
match_kind duplicates information already in QueryProfileDiffScenario - if scenario is 'plans_equal', match_kind is always 'structural', etc
There was a problem hiding this comment.
Agree, plan to remove this for now
| stats: Record<string, QueryProfileDiffStatDelta>; | ||
| } | ||
|
|
||
| export interface QueryProfileDiffPlanComparison { |
There was a problem hiding this comment.
The backend could do graph-based matching instead - actually traversing the plan graph and pairing operators by their role and connections, not just their order. This would solve plans not being structurally identical. Then we could add match_strategy: 'positional' | 'graph' to record how operators were paired. As we will have the backend doing matching, the UI needs to know which was used
There was a problem hiding this comment.
Definitely, this type will be the result of the graph isomorphism problem. The whole type will change once we have more of that strategy figured out I assume.
| unmatched_operator_b_count: number; | ||
| } | ||
|
|
||
| export interface QueryProfileDiffResponse { |
There was a problem hiding this comment.
Add:
shared_resource_types: string[]; // covers the timeline resource type dropdown, server already knows which resource types are shared
|
|
||
| export type QueryProfileDiffTimelineEntries<T> = [T, T, ...T[]]; | ||
|
|
||
| export interface QueryProfileDiffTimelineRequest { |
There was a problem hiding this comment.
Right now we are having the FE build two SingleTImelineRequest objects and pass them up; however, the server already has all this info. Instead we should treat this like we do QueryProfileDiffRequest, here's my suggestion for the revised type once we have the API compute everything:
export interface QueryProfileDiffTimelineRequest {
query_a_id: string;
query_b_id: string;
resource_type: string;
config: TimelineConfig;
}
There was a problem hiding this comment.
We'll need to handle n queries in the comparison requests, so should use the same strategy (whatever is landed on) as QueryProfileDiffTimelineRequest, otherwise agree
| delta_config: TimelineConfig; | ||
| } | ||
|
|
||
| export interface QueryProfileDiffTimelineResponse { |
There was a problem hiding this comment.
Here would be the response proposed for the request above:
export interface QueryProfileDiffTimelineResponse {
timeline_a: SingleTimelineResponse;
timeline_b: SingleTimelineResponse;
delta: SingleTimelineResponse;
warnings?: string[];
}
| } from '@quent/utils'; | ||
|
|
||
| export interface QueryProfileDiffRequest { | ||
| query_a_id: string; |
There was a problem hiding this comment.
This needs to be either an array (with the assumption that the first item is the "baseline"), or have explicit baseline and competitors (or similarly named) properties
| } | ||
|
|
||
| export interface DiffRequest { | ||
| // Alternatively make this one array, assume first item is baseline |
There was a problem hiding this comment.
One thought: We talked about comparing query groups (eg. compare tpch runs from 2 commits), we should think that through and update this to accommodate. Not sure about comparing engines, but probably should handle that as well.
|
POC did its job, replaced by |
3 participants
Building a mock API + UI to iterate on diff API
V0 API types:
https://github.com/johallar/quent/blob/8f986c3536068a47fde938fa841282edefdbc14f/ui/packages/%40quent/client/src/queryProfileDiffTypes.ts
Feel pretty good about the modularization so far, this uses the pivot table, radix, and timeline components to implement the diff view and the biggest component so far is the diff page's layout: