-
-
Notifications
You must be signed in to change notification settings - Fork 143
fix: replace CTEs with joins #586
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
Conversation
src/lib/sql/tables.sql
Outdated
select | ||
n.nspname as schema, | ||
c.relname as table_name, | ||
a.attname as name, | ||
c.oid :: int8 as table_id | ||
from | ||
pg_index i, | ||
pg_class c, | ||
pg_attribute a, | ||
pg_namespace n | ||
where | ||
i.indrelid = c.oid | ||
and c.relnamespace = n.oid | ||
and a.attrelid = c.oid | ||
and a.attnum = any (i.indkey) | ||
and i.indisprimary |
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.
This is just primary_keys.sql
but inlined
select | ||
c.oid :: int8 as id, | ||
c.conname as constraint_name, | ||
nsa.nspname as source_schema, | ||
csa.relname as source_table_name, | ||
sa.attname as source_column_name, | ||
nta.nspname as target_table_schema, | ||
cta.relname as target_table_name, | ||
ta.attname as target_column_name | ||
from | ||
pg_constraint c | ||
join ( | ||
pg_attribute sa | ||
join pg_class csa on sa.attrelid = csa.oid | ||
join pg_namespace nsa on csa.relnamespace = nsa.oid | ||
) on sa.attrelid = c.conrelid and sa.attnum = any (c.conkey) | ||
join ( | ||
pg_attribute ta | ||
join pg_class cta on ta.attrelid = cta.oid | ||
join pg_namespace nta on cta.relnamespace = nta.oid | ||
) on ta.attrelid = c.confrelid and ta.attnum = any (c.confkey) | ||
where | ||
c.contype = 'f' |
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.
This is just relationships_old.sql
but inlined
test/lib/tables.ts
Outdated
"name": "c", | ||
"name": "cc", | ||
"schema": "public", | ||
"table_name": "t", | ||
}, | ||
{ | ||
"name": "cc", | ||
"name": "c", |
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.
We don't guarantee ordering of pkeys, so 🤷
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.
Code changes LGTM - can't really judge the functionality though
I've tested that the output is consistent on |
Can't aggregate over 2 source tables in the same query, otherwise it'll produce incorrect results: https://stackoverflow.com/a/27626358/12396224
af75c19
to
2fcab08
Compare
* fix: replace CTEs with joins * chore: remove unused vars & files * chore: complete group by columns * fix: pre-aggregate primary_keys Can't aggregate over 2 source tables in the same query, otherwise it'll produce incorrect results: https://stackoverflow.com/a/27626358/12396224
Benchmark on 1000 tables, 4 columns each:
Schema:
Workload:
GET /tables?include_columns=false
Before: https://explain.dalibo.com/plan/h044ad2ae694gfc7
After: https://explain.dalibo.com/plan/0a506f9b827e1b4c
Conclusion: 20x speedup by using
group by
instead of aggregate filters.