SQL Code Smells

In this article, I cover SQL code smells: some straightforward dos, don’ts, and gotchas when writing SQL queries, as well as some smells that indicate that the data model can likely be improved.

Dos

• Do use CTEs where appropriate
• Do use explicit JOIN criteria
• Do use alias source for all columns
• Do use column aliases to provide clarity
• Do use a SQL formatter
• Do specify an ELSE in CASE WHEN
• Do use naming conventions
• Do use a filter with DELETE

Don’ts

• Don’t use ORDER BY unnecessarily
• Don’t use = NULL
• Don’t use magic numbers in filters
• Don’t use magic numbers for missing data
• Don’t name columns as keywords
• Don’t use SELECT * in models
• Don’t use DISTINCT as a cheap fix
• Don’t use NOT IN a WHERE clause

SQL is the lingua franca of data, the default data-defining, data-manipulation, and data-querying programming language. While data people use many different tools and languages to manipulate and analyze data, SQL is still the primary language for modeling data and data collection, i.e. the first step of most analyses and machine learning modeling. Although SQL has been around since the 1970s, it shows no sign of going anywhere soon.

For these two reasons — that SQL is here to stay and that every data person need to know and user SQL well— developing good SQL practice early in your career is likely to be a great investment and stand you in good stead for many years to come.

The primary focus here is on SELECT queries, i.e. data people as users or consumers of the data in a data-lake or warehouse, and less on the creation of the intermediate models, warehouse, or analytics-centered SQL interface such as data marts. Those tend to be driven, in part by conventions of the tooling and associated community (such as dbt users), by specific needs (such as GDPR compliance), or by organizational norms.

Long queries and CTEs

At a high-level, writing, or having to write, long complex queries can be an indicator of several potential issues.

Common Table Expressions (CTEs)

If the long query starts with SELECT and not with WITH, this is an indication that the query can probably be made more modular and easier to understand using common table expression or CTEs.

CTEs provides the ability to create more modular chunks of code that pertain to one conceptual result set of interest. These chunks, just like functions in Python or other languages, reduce the scope that the human reader has to understand and reason about in one go.

For instance, suppose you want to run an analysis of users in the north east US who bought a certain suite of products within a certain time frame, this can be broken down into two or more CTEs:

WITH north_east_US_users AS (
    -- reader just needs to focus on user in the N.E. US
    SELECT
      user_id,
      gender,
      age,
      ...
    FROM users
    WHERE state_code IN ('CT', 'ME', 'VT',...)
),

crunchy_snack_orders AS (
     -- reader can forget users for now and focus on order and product type
     SELECT
       order_id,
       user_id,
       product_name,
       quantity,
       order_date
    FROM orders
    WHERE product_category IN ('Chips', 'Crackers',...)
    AND order_date >= '2024–01–01'
)

This makes it much easier for the reader to understand the focal user subpopulation (first CTE) as one small chunk before moving onto the second chunk which covers order product type.

Not only that but CTEs make development easier. That is, you can write the first CTE and then run that alone:

WITH north_east_US_users AS (
    SELECT ...
)
SELECT * FROM north_east_US_users

or

WITH north_east_US_users AS (
    SELECT ...
)
SELECT count(*) FROM north_east_US_users

to verify that you are getting what you expect, or to provide some expectations of the result set will contain.

Then you can add in the second CTE but run only the second CTE:

WITH north_east_US_users AS (
    SELECT ...
),

crunchy_snack_orders AS (
    SELECT ...
)

SELECT * FROM crunchy_snack_orders
LIMIT 50

This will allow you to build up the complexity, validating as you go, rather than writing one giant raw query and hoping for the best.

Like functions, CTEs also allow for code reuse. That is, the CTE might be used in multiple places in the query. Suppose a store has different sales table for in-store purchases versus web sales. If the analysis is focussed on the same subpopulation of north east users, then that CTE can be joined into the in store sales table and into the web sales data, reusing the same CTE.

WITH north_east_US_users AS (
    SELECT...
),

web_orders AS (
    SELECT
        order_id,...
    FROM web_orders wo
    INNER JOIN north_east_US_users u -- modular CTE payoff
    ON wo.user_id = u.user_id
    ...
),

in_store_purchases AS (
    SELECT
        order_id,...
    FROM in_store_purchases isp
    INNER JOIN north_east_US_users u -- + code reuse payoff
    ON isp.user_id = u.user_id
    ...
),
...

This adheres to keeping code DRY (Developer best practice: Don’t Repeat Yourself). Less code to write, fewer places to make changes.

As you have to name your CTEs, this provides another opportunity to make good choices and provide context, which again aids the reader (Developer best practice: be kind to your future self).

Complex business logic

If, even after using CTEs and making the query as clear and modular as possible, you find your self often having to repeat the same sort of business logic (copy and pasting from another query is a definite code smell here), it may mean that there is an opportunity to provide an upstream intermediate model to encapsulate that business logic once. This is also true if you find yourself writing business logic to implement a metric. That is a probably a bad smell indicating that the metric could or should be defined better in the data marts or dim or fact tables.

Those might not been in your control, possibly being in the domain of the analytics engineers, but I would always encourage you to have a conversation with that team. Show them your queries and what you are struggling with. In my experience, these teams are typically eager to help users, they understand the data model best (often providing an alternative, better way to get at a dataset), or if the model generally can be improved, will add your request to their backlog.

In the next two sections, we list some practical dos and don’ts. They are not set in stone. Instead, treat them as general guidelines as there are always exceptions but in general, they are tried-and-true best practice and represent sound advice.

Dos

Do use CTEs where appropriate

As discussed above, CTEs can provide modularity, clarity and context, and code reuse. They are typically easier to write, debug, and validate than a raw large query without CTEs.

Do use explicit JOIN criteria

SQL will allow JOINs without explicit join types:

SELECT * FROM a, b

which is a cross join, or

SELECT * FROM a JOIN b

which is an inner join.

However, following the principle that explicit is better than implicit, it is better to specify the JOIN type and join criteria. Thus, while longer

SELECT
   *
FROM a
INNER JOIN b ON a.user_id = b.user_id

is preferable than

SELECT
*
FROM a JOIN b

where the reader has to interpret or guess both the join types and common fields. Clarity trumps brevity here.

Do use alias source for all columns

There are many occasions where your SQL engine will require that you use an alias, such as to specify an ON clause in a join, or to disambiguate a field in a SELECT that appears in two or more tables of a join.

However, there are times where you have access to an alias, due to the JOIN requirements, but where columns are not ambiguous — at least to the SQL engine. While it might be obvious to you, right here, right now, which columns come from which table, that is not necessarily true in a week’s time, or for someone else reading your code who is less familiar with the tables.

Thus, it is better to specify the source of those columns for all columns in the select:

SELECT
  -- every column is specified with u. or o.
  u.user_id,
  u.gender,
  u.age,
  o.product_name,
  o.product_category,
  ...
FROM users u INNER JOIN orders o
ON u.user_id = o.user_id

Do use column aliases to provide clarity

Aliases at the columns in the select provide an opportunity to rename columns to reduce confusion and provide clarity. Use them to your advantage.

Suppose you are joining users, which contain a home address, and orders, which contain a shipping address and each address has a state column. While the SQL engine will require that you use a table alias to specify which table the column comes from:

SELECT
  u.suer_id,
  o.state, -- as opposed to u.state
  o.product_name,
  o.product_category
FROM users u INNER JOIN orders o
ON u.user_id = o.user_id

adding column-level renaming with an alias can greatly aid clarity:

SELECT
  u.suer_id,
  u.state AS home_address_state, -- no confusion here
  o.state AS shipping_address_state, -- no confusion here
  o.product_name,
  o.product_category
FROM users u INNER JOIN orders o
ON u.user_id = o.user_id

Clear, easily understood code is the goal. If it reduces confusion and that saves human time or avoids bugs, then the few extra minutes in thinking time and few extra keystrokes are certainly worth it.

Do use a SQL formatter

Use a SQL formatter or better yet a SQL linter that also formats your code (such as sqlfluff). Establish a process that this tool is run easily, either through format on save in your IDE, or through pre-commit hooks.

Formatting is not about looking professional and pretty but the formatting rules are optimized for code clarity and so there is some standard at work. That in turn will make git diffs minimal in you pull requests.

Do specify an ELSE in CASE WHEN

While writing a CASE WHEN, always specify a default result in an ELSE. The SQL language does not require you to specify an ELSE. If you don’t, it will produce a NULL. Cover all branch logic explicitly.

CASE
    WHEN product_type ilike '%chip%' THEN 'Chips'
    WHEN product_type llike '%cracker%' THEN 'Crackers'
-- ❌ what happens if it doesn't match one of these two cases?
END AS product_category

Instead, this is preferred:

CASE
    WHEN product_type ilike '%chip%' THEN 'Chips'
    WHEN product_type llike '%cracker%' THEN 'Crackers'
ELSE
    NULL -- ✅ this is much clearer intent
END AS product_category

Related, it can be good practice to explicitly list all the categories of interest (one WHEN for each) letting others to fall into a default bucket. Consider the following:

CASE
    WHEN state_code in ('CT','ME','VT',...) THEN 'NORTH_EAST'
    WHEN state_code in ('FL','GA','SC',...) THEN 'SOUTH_EAST'
    WHEN state_code in ('WA','OR','ID',...) THEN 'NORTH_WEST'
    ELSE
        'SOUTH_WEST' -- ❌ not recommended, see below
ELSE
END AS us_region

At first sight, this seems reasonable. Anything that is not N.E., S.E., or N.W. must be S.W. right? That’s fine until you realize you forgot Alaska, Hawaii, or Puerto Rico. The SOUTH_WEST will collect anything outlying.

Thus, the following version where each region is explicitly specified, will do a better job at identifying anything that has been missed. Examining those in the OTHER categories provides an opportunity to update the logic as necessary. This is much safer than a “polluted” SOUTH_WEST category where you might not realize that there is even an issue.

Really, the worst case scenario with this more specified strategy is that you have captured all the WHEN logic and this final default category is empty. If that is the case, that is the basis of a good unit test — alert me if anything does fall into this category.

CASE
    WHEN state_code in ('CT','ME','VT',...) THEN 'NORTH_EAST'
    WHEN state_code in ('FL','GA','SC',...) THEN 'SOUTH_EAST'
    WHEN state_code in ('WA','OR','ID',...) THEN 'NORTH_WEST'
    WHEN state_code in ('CA','TX','AZ',...) THEN 'SOUTH_WEST'
    ELSE
        'OTHER' -- ✅ 'AK', 'HI', 'PR','UM','MP' etc
ELSE
END AS us_region

Do use naming conventions

Adopt the naming conventions of your team and do so consistently. If they don’t have a good convention, or you are on your own (perhaps as the first data hire), then set one out, lead by example.

Get Carl Anderson’s stories in your inbox

Join Medium for free to get updates from this writer.

Remember me for faster sign in

For instance,

• Are models singular or plural: order or orders?
• What prefix set does your team use: dim_ for dimensional, fct_ for fact, stg_ for staging etc.?
• What suffixes do you use, if any (__daily, __weekly?)
• Is there a convention for prefixes on table constraints, e.g. pk for primary key, fk for foreign key etc.
• Are words of names separated by underscores or do they run on, e.g. customer_id vs CustomerID? I would suggest the former as they are clearer and less ambiguous.
• Are the primary keys of tables always id or do they specify the nature of the entity, e.g. customer_id. I would suggest the latter as it is less ambiguous when those are referenced in other tables; you know it is a foreign key and from which table it stems.
• While you should generally avoid abbreviations, are there accepted exceptions in your domain and codebase?

(In this area, I would recommend Tom Benner’s book: Naming Things: The Hardest Problem in Software Engineering.)

Following a consistent pattern will help others, and your future self, understand the code (what is happening where), and if done well, contribute to the principal of least surprise. Like code formatting, the particular choices for the standard are less important than a standard exists and everyone follows it.

In summary, keep it simple: short, unambiguous descriptive names, with no special characters, that accurately convey the nature of the data or model.

Do use a filter with DELETE

If deleting rows, always make sure that some filter exists. Deleting all rows of a table is not uncommon but for that you would use TRUNCATE TABLE or DROP TABLE. If you are using DELETE, then that means you only want to delete a subset set of records so always always have a filter. If that filtering clause is not simple, then test it. Turn the query into a SELECT to make sure you are getting the records would expect.

Don’ts

Don’t use ORDER BY unnecessarily

A good data model provides a table or relation that can be used in many different analyses. While it is tempting to order the table to make it clean and logical when visually inspecting the data, generally this will be a waste of compute. The table or view is likely to be filtered or joined in many different ways such that any ordering becomes irrelevant. Let the downstream consumers define their own ordering via ORDER BY, a window function, or QUALIFY.

Ordering data is typically expensive, especially across multiple columns with high cardinality or continuous data types (such as float). Instead, rely on the databases indexing or partitioning to do their magic.

There are many exceptions of course. Incremental models and time series data are typically ordered chronologically as new data arrive. Small reference tables (such as countries of the world or US states) that at are created once, rarely updated, are likely to be sorted at ingest. The point I am making is don’t order data to make it look pretty. Order it because it is necessary or a natural consequence of ingest, or matches user expectations of usability.

Don’t use = NULL

When comparing to NULL use IS or IS NOT and not =, !=, or <>. While the latter might work as intended, they probably will not. Typically, it will be undefined or False. It depends on the SQL engine. However, IS and IS NOT are guaranteed to be well-behaved.

Don’t use magic numbers in filters

Try to avoid special numbers in code whose meaning isn’t obvious to the reader, can be misconstrued, or represents a maintenance burden.

Suppose you have a small dimensional table with types of movie genres:

movie_genre_id,genre_name
1,thriller
2,drama
3,horror
6,comedy
...

it will be hard for a reader of a query to determine what is meant by:

SELECT
    ...
LEFT JOIN movie_genre g ...
WHERE
    g.movie_genre_id in (2,4,7) ❌ -- what do these represent?

They’ll have to run a query to look those up. Worse, imagine you had financial quarters as

quarter_id,quarter_name
0,'Q1'
1,'Q2'
2,'Q3'
3,'Q3'

then someone seeing

WHERE
    q.quarter_id in (1,2) ❌ -- is this Q1 and Q2?

might incorrectly assume that they are Q1 and Q2. This is an example of Principle of Least Surprise and would be a bad design choice.

In both cases, use the values themselves, such as:

WHERE
    g.genre_name in ('drama','western','action') ✅ -- clear intent

⚠️ There is some small risk with filtering on the values. Both the IDs or values in that foreign table might change but values are more likely to change than IDs. In a relational database, the IDs are probably bound by a foreign key constraint but the values have no such constraints. For instance, there might be a typo that needs to be a fixed or a change in the presentation logic for these values. ⚠️

With modern cloud columnar warehouses, such as Snowflake and BigQuery, it is now much more common to work with wider, denormalized tables, avoiding some of these issues.

In the two cases above, the meaning of those values may not be obvious but it is clear where they are coming from and how to determine them. However, what about:

WHERE user_id
NOT IN
    ('cebf6457-74c9-4e2f-801d-f32fa2f0a8cb',
    '5a669229-6ee4-48e3-9398-48619dec9358') ❌ -- what do these represent?

In this case, what do those users represent? Test users? Some users with special privileges? It may not be obvious where to answer that question.

If you can’t modify the database model to provide more context, such as an is_test_user boolean flag, you can at least provide a docstring, or better yet a CTE:

WITH test_users AS ( ✅ -- more context
    SELECT
    'cebf6457-74c9-4e2f-801d-f32fa2f0a8cb' AS user_id
    UNION
    SELECT
    '5a669229-6ee4-48e3-9398-48619dec9358' AS user_id
)
...

WHERE user_id
NOT IN (SELECT user_id FROM test_users)

Don’t use magic numbers for missing data

Related to above, let NULL denote missing data. Don’t set special values such as 999, -1, or “unknown” into the source model or data itself. The analyst can fillna() in pandas if they need to but let them make that choice.

It adds a burden downstream to the consumer data scientist or analysts writing queries against it. They’ll have to add filtering logic to filter out those values. Of course, they might need to do the same for NULL but that is generic. 999 needs domain knowledge of how those data were generated or processed.

Another risk with such a strategy is that you might set a value which could be an actual value and you can’t disambiguate a known value versus a missing value. This is especially true with 0. Or, maybe you don’t know or forget about these special values and blindly do an average, confused why the metric is high, pulled up by these 999s. NULLs can be a pain to work with sometimes but they are preferable to magic values.

Don’t name columns as keywords

Many databases have restrictions on how you may name columns or tables. You might not be able to start with a number or may not be allowed spaces. However, there is still a lot of scope to get you into trouble or cause confusion.

Look at the situation of this poor StackOverflow user:

One of my columns is called `from`. I can’t change the name because I didn’t make it. Am I allowed to do something like `SELECT from FROM TableName` or is there a special syntax to avoid the SQL Server being confused?

At a minimum, any syntax highlighter will be confused and color those differently. In many cases you will need to double quote the identifier or add other structure such as square brackets []. It adds friction.

Databases’ list of reserved words may contain words that may relate to your domain, such as account, order, or sample. Try to avoid using those and use more descriptive or specific words or phrases instead; such as account_name, order_number, or product_sample.

Don’t use SELECT * in models

The wildcard *, as in SELECT *, can be a huge timesaver to select all the columns, especially when joining two or more wide tables. This is especially true when your database (such as Snowflake and DuckDB) provides EXCLUDE or some other keywords to select columns that you don’t want:

SELECT
  a.*,
  b.*
  EXCLUDE b.id
FROM wide_table_a AS a
INNER JOIN wide_table_b AS b
ON a.id = b.id

For analyses, i.e. where you are the only user, this can be fine. However, if building a data model in a warehouse or a dbt project where they can be downstream users, this is a bad idea.

Why is that? By using SELECT *, the output is prone to any schema changes in the source tables. Processes outside of your control might

• add a column
• delete a column
• rename a column.

If your query blindly grabs those columns and passes them along, it can break downstream processes.

It would break, for instance:

• if they are UNIONing data from two or sources
• if they are grouping by column numbers (e.g. GROUP BY 1, 2, 3) — which, in my opinion, is bad form.
• if they are inserting data into a table or other sink that expects a certain schema
• if they are being fed into a pickled machine learning model

So, there are lots of situations which expect a given, fixed schema.

To be clear, you can use SELECT * in a model, just don’t make it the last query of the model. That is, this is fine

WITH my_first_cte AS (
    SELECT
        *
    FROM table_a AS a
    INNER JOIN table_b AS b
    ON a.id = b.id
),

my_second_cte AS (
    SELECT
        *
    FROM table_c AS c
    WHERE week = (SELECT max(date) from table_c)
)

SELECT
  f.col_1,
  f.col_2,
  s.col_3
FROM my_first_cte f
INNER JOIN my_second_cte s
ON f.id = s.id

because the output is explicit and fixed. Any additional columns added to tables a, b, or c will not affect the final output.

One counter-example I have seen — and which my team mates have grumbled about in peer review any time I used it — is in dbt models. The advice and dbt convention is to write everything in CTEs and the final line is a SELECT * from the last CTE:

{{ config(
   materialized="view",
   schema="staging"
) }}

WITH nj_sales AS (
    SELECT
       customer_id,
       SUM(total_price) AS total_price,
   FROM {{ ref("orders") }}
   WHERE state_code = 'NJ'
   GROUP BY customer_id
)

SELECT
   * -- ❓ anti-pattern?
FROM nj_orders

Their argument, same as my CTE discussion above, is that it helps with development and debugging of code. Still, this is only acceptable if the prior CTE specifies each column explicitly in its SELECT.

As my former boss used to say (when I used this pattern in a dbt model):

wildcards are evil. I would rather see the list

Don’t use DISTINCT as a cheap fix

If your query is producing duplicates (multiple exact copies of a row), an easy, quick fix is to add a DISTINCT:

SELECT
    DISTINCT
    col_1,
    col_2,
    col_3
FROM table_a...

While that might sometimes be the best approach, don’t do so because it is quicker and easier than taking the time to understand why this is happening.

If dupes are being introduced in one of your earlier CTEs, it would be better to dedupe at that stage. First, earlier filtering means fewer later rows to process (although often the query planner can figure this out for you). Second, the deduping code at the source CTE is a form of documentation of what processing needs to be performed with that table or view to make it usable. That will aid others and your future self when reading the code.

Don’t use NOT IN a WHERE clause

Avoid filtering by using a NOT IN in a WHERE clause.

If the condition is short and hard-coded, such as

...
WHERE product_type NOT IN ('Chips', 'Crackers') -- ✅ OK

then this may be fine. However, if the condition involves a sub query:

...
WHERE customer_id NOT IN
    (SELECT customer_id
    FROM customers
    WHERE state='NJ') -- ❌ inefficient

then this could result in a full table scan and so is less efficient.

There are a few options to counter this but the most obvious is a LEFT JOIN:

WITH nj_customers AS (
    SELECT customer_id
    FROM customers
    WHERE state = 'NJ'
)
SELECT
    ...
FROM customers c
LEFT JOIN nj_customers -- ✅ more efficient
ON c.customer_id = nj_customers.customer_id
WHERE nj_customers.customer_id IS NULL

If in doubt, try both and use an EXPLAIN and examine the execution plans.

Related, if you do use WHERE x NOT IN (…), ensure that the list elements do not contain NULL. If they do, they this may not produce the results that you expect. Similar to = NULL versus IS NULL above, SQL has a hard time to compare something with NULL, which is the absence of something. The result is most likely be undefined or unknown. Logically filter your data so that all values to be compared to are non-NULL.

So there you have it: an opinionated list of dos and don’t that should produce higher quality SQL code for you, your future self, and your teammates.