Skip to content

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

Merged
merged 4 commits into from
Jun 8, 2023
Merged

fix: replace CTEs with joins #586

merged 4 commits into from
Jun 8, 2023

Conversation

soedirgo
Copy link
Member

@soedirgo soedirgo commented Jun 7, 2023

Benchmark on 1000 tables, 4 columns each:

Schema:

do $$
    DECLARE myvar integer;
begin
    for myvar in 1..1000 loop
        EXECUTE format('CREATE TABLE sbtest%s (
        id SERIAL NOT NULL,
        k INTEGER NOT NULL,
        c CHAR(120) NOT NULL,
        pad CHAR(60) NOT NULL,
        PRIMARY KEY (id))', myvar);
    end loop;
end; $$

Workload: GET /tables?include_columns=false

Before: https://explain.dalibo.com/plan/h044ad2ae694gfc7

  Time (mean ± σ):      8.886 s ±  0.220 s    [User: 0.100 s, System: 0.022 s]
  Range (min … max):    8.666 s …  9.201 s    10 runs

After: https://explain.dalibo.com/plan/0a506f9b827e1b4c

  Time (mean ± σ):     404.1 ms ± 103.5 ms    [User: 94.7 ms, System: 15.1 ms]
  Range (min … max):   352.2 ms … 697.2 ms    10 runs

Conclusion: 20x speedup by using group by instead of aggregate filters.

@soedirgo soedirgo requested review from a team as code owners June 7, 2023 14:05
Comment on lines 32 to 47
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
Copy link
Member Author

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

Comment on lines +51 to +73
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'
Copy link
Member Author

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

Comment on lines 468 to 473
"name": "c",
"name": "cc",
"schema": "public",
"table_name": "t",
},
{
"name": "cc",
"name": "c",
Copy link
Member Author

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 🤷

Copy link
Contributor

@kevcodez kevcodez left a 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

@soedirgo
Copy link
Member Author

soedirgo commented Jun 7, 2023

I've tested that the output is consistent on /tables and /generators/typescript between this and the one on prod, so I think we're good 👍

Can't aggregate over 2 source tables in the same query, otherwise it'll
produce incorrect results: https://stackoverflow.com/a/27626358/12396224
@soedirgo soedirgo force-pushed the fix/replace-cte-with-joins branch from af75c19 to 2fcab08 Compare June 8, 2023 03:54
@soedirgo soedirgo merged commit 166fda0 into master Jun 8, 2023
@soedirgo soedirgo deleted the fix/replace-cte-with-joins branch June 8, 2023 03:58
avallete pushed a commit that referenced this pull request May 13, 2025
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants