-
Notifications
You must be signed in to change notification settings - Fork 163
feat(zql): batch flip join requests #5928
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
20e8618
71297b5
9a97e9f
874684a
c1a2b54
b4551f1
e2384b3
cf12901
86564c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,8 +55,8 @@ describe('Chinook planner tests', () => { | |
| .whereExists('genre', q => q.where('name', 'Rock')), | ||
| ); | ||
|
|
||
| expect(pick(ast, ['where', 'conditions', 0, 'flip'])).toBe(true); | ||
| expect(pick(ast, ['where', 'conditions', 1, 'flip'])).toBe(false); | ||
| expect(pick(ast, ['where', 'conditions', 0, 'flip'])).toBe(false); | ||
| expect(pick(ast, ['where', 'conditions', 1, 'flip'])).toBe(true); | ||
|
Comment on lines
+58
to
+59
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, why would we flip genre over album now? Maybe costs are about equal so it just picks one at random?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This flip cost change caused planner benchmarks to regress 30x. |
||
| }); | ||
|
|
||
| test('playlist with track', () => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,7 +56,7 @@ describe('Pagila planner execution cost validation', () => { | |
| .where('district', 'California') | ||
| .whereExists('city', c => c.whereExists('country')), | ||
| validations: [ | ||
| ['correlation', 1], | ||
| ['correlation', 0.8], | ||
| ['within-optimal', 1], | ||
| ['within-baseline', 1], | ||
| ], | ||
|
|
@@ -95,9 +95,9 @@ describe('Pagila planner execution cost validation', () => { | |
| .whereExists('actors', a => a.where('lastName', 'GUINESS')) | ||
| .whereExists('categories', c => c.where('name', 'Action')), | ||
| validations: [ | ||
| ['correlation', -0.09], | ||
| ['correlation', 0.5], | ||
| ['within-optimal', 1], | ||
| ['within-baseline', 0.07], | ||
| ['within-baseline', 0.046], | ||
| ], | ||
| }, | ||
|
|
||
|
|
@@ -124,8 +124,10 @@ describe('Pagila planner execution cost validation', () => { | |
| i.whereExists('film', f => f.where('rating', 'PG')), | ||
| ), | ||
| validations: [ | ||
| ['correlation', 0.4], | ||
| ['within-optimal', 2.1], | ||
| // TODO: why decreased correlation? From 0.4 to -0.1 | ||
| // 'payment to film via rental chain (3 hops)' sees a huge improvement though. | ||
| ['correlation', -0.1], | ||
| ['within-optimal', 2.0], | ||
| ['within-baseline', 1], | ||
| ], | ||
| }, | ||
|
|
@@ -143,18 +145,13 @@ describe('Pagila planner execution cost validation', () => { | |
| ) | ||
| .limit(100), | ||
| validations: [ | ||
| ['correlation', -0.5], | ||
| // Correlation is bad for this one and we chose a non-optimal query. | ||
| // What the below is saying is that the query as written is most optimal | ||
| // (within-optimal and within-baseline are the same). | ||
| // On inspection of the data, however, the chosen plan (everything flipped) should be close to optimal. | ||
| // The issue is that walking back up the tree (from most nested exists check) is missing indices. | ||
| // | ||
| // 1. payment table has NO index on rental_id | ||
| // 2. inventory table has NO index on film_id | ||
| // We need to come up with a good way to penalize `SCAN` vs `SEARCH` | ||
| ['within-optimal', 10], | ||
| ['within-baseline', 10], | ||
| // Big improvement after multi-IN AND propagation across chained | ||
| // FlippedJoins — the missing indexes that previously forced a | ||
| // SCAN are now amortized across one combined IN-list query. | ||
| // Tightened from -0.5 / 10 / 10. | ||
| ['correlation', 0.9], | ||
| ['within-optimal', 1], | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wow, this is a big win! |
||
| ['within-baseline', 0.47], | ||
| ], | ||
| }, | ||
|
|
||
|
|
@@ -184,9 +181,9 @@ describe('Pagila planner execution cost validation', () => { | |
| ), | ||
| ), | ||
| validations: [ | ||
| ['correlation', 0], | ||
| ['correlation', 0.75], | ||
| ['within-optimal', 1], | ||
| ['within-baseline', 1], | ||
| ['within-baseline', 0.85], | ||
| ], | ||
| }, | ||
|
|
||
|
|
@@ -198,8 +195,8 @@ describe('Pagila planner execution cost validation', () => { | |
| ) | ||
| .limit(100), | ||
| validations: [ | ||
| ['correlation', 0], | ||
| ['within-optimal', 1], | ||
| ['correlation', 0.5], | ||
| ['within-optimal', 1.9], | ||
| ['within-baseline', 1], | ||
| ], | ||
| }, | ||
|
|
@@ -222,8 +219,10 @@ describe('Pagila planner execution cost validation', () => { | |
| name: 'high fanout - store inventory', | ||
| query: queries.store.where('id', 1).whereExists('inventory'), | ||
| validations: [ | ||
| ['correlation', -1], | ||
| ['within-optimal', 1.4], | ||
| // Tightened from -1 / 1.4 after multi-IN AND propagation lets | ||
| // the source filter both join keys in one query. | ||
| ['correlation', 0.9], | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤩 |
||
| ['within-optimal', 1], | ||
| ['within-baseline', 1], | ||
| ], | ||
| }, | ||
|
|
@@ -239,7 +238,7 @@ describe('Pagila planner execution cost validation', () => { | |
| ) | ||
| .limit(5), | ||
| validations: [ | ||
| ['correlation', 0.4], | ||
| ['correlation', 0.2], | ||
| ['within-optimal', 1], | ||
| ['within-baseline', 1], | ||
| ], | ||
|
|
@@ -272,7 +271,7 @@ describe('Pagila planner execution cost validation', () => { | |
| // within-optimal excluded: empty results cause divide-by-zero (optimal has 0 rows) | ||
| validations: [ | ||
| ['correlation', 0.8], | ||
| ['within-baseline', 0.01], | ||
| ['within-baseline', 0], | ||
| ], | ||
| }, | ||
|
|
||
|
|
@@ -298,9 +297,10 @@ describe('Pagila planner execution cost validation', () => { | |
| .whereExists('language', l => l.where('name', 'English')) | ||
| .whereExists('actors', a => a.where('lastName', 'BERRY')), | ||
| validations: [ | ||
| ['correlation', 0.77], | ||
| ['correlation', 0.74], | ||
| // TODO | ||
| ['within-optimal', 31], | ||
| ['within-baseline', 0.2], | ||
| ['within-baseline', 0.19], | ||
| ], | ||
| }, | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
most of all the planner statistics got better which is good news.