Do not merge lines with more than N segments #334
Replies: 2 comments 2 replies
-
I think segments is key, but I think the complexity of each segment matters too select * from table where x is not null feels like a query I'd be ok with one-lining, and it also has 3 top-level segments. the fact that your example the |
Beta Was this translation helpful? Give feedback.
-
Reviving this thread: Being contrarian on this 8) For CTEs: with
-- import datasets
orders as (
select *
from {{ source("db", "orders") }}
where deleted = 'N' and {{ var("from_date") }} <= order_date
),
blocked_orders as (select * from {{ source("db", "blocked_orders") }}),
instrument as (
select *
from {{ source("db", "instrument") }}
where current_row = 'Y' and deleted = 'N'
)
... Preferred: with
-- import datasets
orders as (
select *
from {{ source("db", "orders") }}
where deleted = 'N'
and {{ var("from_date") }} <= order_date -- indent for 2:+ filter
),
blocked_orders as ( -- same structure as the rest of other CTEs
select *
from {{ source("db", "blocked_orders") }}
),
instrument as (
select *
from {{ source("db", "instrument") }}
where current_row = 'Y'
and deleted = 'N'
)
... For joins: ...
consolidate_data as (
select *
from orders o
join blocked_orders bo on bo.blocked_orders_ek = o.blocked_orders_ekr
join
instrument i
on o.instrument_ekr = i.instrument_ek
and i.start_date <= o.order_date
and o.order_date < i.end_date Preferred: ...
consolidate_data as (
select *
from orders o
join blocked_orders bo
on bo.blocked_orders_ek = o.blocked_orders_ekr -- join keys on next line
join instrument i -- always have joining table next to "join" code
on o.instrument_ekr = i.instrument_ek
and i.start_date <= o.order_date
and o.order_date < i.end_date Tech stack: Snowflake and dbt Cloud Would be great to discuss this further on and finding a balance in the segmenting approach. |
Beta Was this translation helpful? Give feedback.
-
We aggressively merge lines if they fit, even if those lines are very complex:
I'm not sure this is a good idea. The example above has 3 top-level segments (select, from, where) and the
select
segment has 4 segments. Maybe we should check the number of segments (either after we split into segments or before merging them) and skip merging across segments if there are more than N (where N is probably 3-4).Beta Was this translation helpful? Give feedback.
All reactions