For over 16 years, various segments of the SQL Server community have taken part in T-SQL Tuesday, a monthly event where bloggers are loosely guided on a topic of the host's choice. This month, Brent Ozar has asked what we might see during a code review that immediately tells us, "this code is going to be bad."

I was on vacation last week, so I took advantage of the late start afforded by the original invite (post on the 7th or the 14th).

I've looked at a lot of questionable code over my career, and I've spent years ranting about bad habits to anyone who will listen. A few years ago, I had fun with a post about finding 40 problems in a stored procedure, and across two decades I have written about dozens and dozens of code smells that make my spidey-sense tingle. But Brent's requirement seems pretty clear: name one thing that makes us dread the rest of the review. It took me a hot minute to pick just one:

The WRONG JOIN

The headline is probably unfair and is not meant to imply that a RIGHT OUTER JOIN is wrong. But when I see a RIGHT OUTER JOIN, my first thought is, "the rest of this review will probably be harder than it needs to be." I find that it makes queries harder to read, because most people naturally read queries left-to-right. With a left join, the "important" table is on the left, and the query is saying, "give me everything from this table, and maybe something from this related table." With a right join, I have to mentally flip things around to understand which table is important. Think about this query:

SELECT c.<cols>, 
       SUM(o.OrderTotal)
  FROM dbo.Customers AS c
  LEFT OUTER JOIN dbo.Orders AS o
    ON c.CustomerID = o.CustomerID
 WHERE c.<something> ...

For me, this query says:

Give me all customers and, for those who have placed orders, include the total of all their orders.

If you flip that around:

SELECT c.<cols>, 
       SUM(o.OrderTotal)
  FROM dbo.Orders AS o
 RIGHT OUTER JOIN dbo.Customers AS c 
    ON o.CustomerID = c.CustomerID
 WHERE c.<something> ...

That query is semantically equivalent, generates the same execution plan, and has identical performance. But if I read it in order, it says to me:

Give me the order total for all customers that have placed one or more orders and, oh yeah, also give me all the customers that haven't placed any orders.

This is just more mental gymnastics to me – if you care about customers both with and without orders, why would you reference the Orders table first? I don't mentally model joins as "preserved" versus "non-preserved" tables; I read them left-to-right. A left join lets me continue reading naturally. A right join forces me to stop, mentally swap the tables, then continue. Every time I have encountered this type of query, I have rewritten it to be a left join (or inner join, if the predicates make it so anyway, which is quite common). If it's hard to rewrite a right join to a left join (think sequences like INNER / LEFT / INNER / RIGHT), it's probably a sign that the query is needlessly complex, and isn't "bad" solely because it contains a RIGHT OUTER JOIN.

(And yes, I always include the OUTER keyword. It may be optional to SQL Server, but I don't think it's optional to the reader. I want INNER JOIN and OUTER JOIN to look different, because they are different.)

Now, I'm not saying there is no place for a RIGHT OUTER JOIN, just like I would never say there is no place for driving a car on the left or right side of the road. There is nothing inherently wrong with either one, unless you've spent your whole life learning and doing the other.

Imagine if any town or even neighborhood could decide if their local convention was to drive on the other side of the road. Or if a rental car chain arbitrarily assigned both left- and right-hand drive vehicles. The choice itself is not the problem, it's the inconsistency and how it changes the experience of everyone around. Like having a codebase that usually uses left joins but sometimes uses right joins for no clear and obvious reason. Left joins have simply become the "drive on the left/right" convention for SQL, likely for these consistency and readability reasons.

Another analogy you might think of is left-handed scissors. I myself have asked, "what's the difference?" Being surrounded by a disproportionate number of southpaws in my own family, I know it's not a marketing gimmick lifted from The Simpsons: the differently-angled blades solve a real ergonomic problem that most of us haven't experienced. If you're left-handed, I'm sure you already know that most scissors in this world are cumbersome to use. If you are right-handed, I urge you to try out a pair of left-handed scissors; you will likely find them harder to use effectively. They still work; any any-handed person can use any any-handed scissors to cut paper but, like suddenly encountering a right join in a sea of inner and left joins, it's awkward.

Okay, back to SQL…

If your entire team is fully on board with using right joins everywhere, that's cool. But if you're not always writing right joins, or not everyone has bought in, then there's inconsistency in the codebase. Inconsistency makes code reviews bad for additional reasons and, as your codebase grows, that can multiply. If you are writing or reviewing a query with a RIGHT OUTER JOIN, ask yourself why, and whether it will be intuitive or "normal" to the next reader. Right joins are not wrong, they're just not the established convention.

I understand why the syntax exists: it offers language symmetry and can make generated SQL or query transformations easier. But that doesn't mean it's the clearest choice for humans reading hand-written code.

Bonus material

A right join often tells me I'm about to review a query that's more complicated than it needs to be. But there are several additional things that give me the same kind of pause and the same dread of a future teaching moment:

• NOLOCK (especially on UPDATEs)
• MERGE (especially without HOLDLOCK)
• scalar UDFs (especially in the WHERE clause)
  • bonus: that read from the registry
    • mega bonus: that write to the registry
• NOT IN (<any query at all>)
• BETWEEN for date range predicates
• regional / ambiguous date formats
• assumptions about @@DATEFIRST, @@LANGUAGE, or case sensitivity
• non-contextual table aliases like a, b, c
• date/time shorthand

There are also things that don't immediately give me this feeling. These might be big red flags for some of my colleagues, but you can use them effectively and they should never be an automatic cause for concern without additional context:

• temp tables (I'll take them over table variables every time)
• cursors (they're not always unnecessary)
• dynamic SQL (but only when necessary and only with protection!)
• recursive CTEs (just not for generating a set of numbers or dates!)