DEV Community: Martin Vielsmaier

SQLAlchemy: Prevent implicit cross join

Martin Vielsmaier — Wed, 01 Jan 2020 23:00:00 +0000

I really like to create SQL queries using SQLAlchemy’s explicit and declarative API. When using this instead of raw strings, roughly half of errors I tend to introduce are caught before even sending the query.

Here is an example:

import sqlalchemy as sa

metadata = sa.MetaData()
a = sa.Table(
    "a",
    metadata,
    sa.Column("a_id", sa.Integer, primary_key=True, autoincrement=True),
    sa.Column("name", sa.String),
)
b = sa.Table(
    "b",
    metadata,
    sa.Column("b_id", sa.Integer, primary_key=True, autoincrement=True),
    sa.Column("a_id", sa.Integer, sa.ForeignKey(a.c.a_id)),
)

def create_select(additional_filters):
    return sa.select([a], whereclause=sa.and_(*additional_filters))

print(create_select([a.c.name == 'Foo']))
# SELECT a.a_id, a.name
# FROM a
# WHERE a.name = :name_1

A huge advantage over dealing with string queries is that you can create different parts of the query on their own and combine them. In the example I create the SELECT statements in a central place and allow to pass in parts of the where clause.

Unfortunately, this pattern has a dangerous property: It will implicitly add aCROSS JOIN when you pass a filter expression that contains a column from a table that is not part of the select statement already.

print(create_select([a.c.name == 'Foo', b.c.b_id == 1]))
# SELECT a.a_id, a.name
# FROM a, b
# WHERE a.name = :name_1 AND b.b_id = :b_id_1

On small tables this will just create an unexpected result set, but when the involved tables are large this query might well exhaust the DB server’s resources. The concrete problem can be fixed by adding a join to all the tables that should be allowed in the filters:

def create_select_corrected(additional_filters):
    return sa.select([a], from_obj=a.join(b), whereclause=sa.and_(*additional_filters))
print(create_select_corrected([a.c.name == 'Foo', b.c.b_id == 1]))
# SELECT a.a_id, a.name
# FROM a JOIN b ON a.a_id = b.a_id
# WHERE a.name = :name_1 AND b.b_id = :b_id_1

A more abstract problem is that we can create queries that will lead to unexpected results without noticing. A good but pretty expensive solution would be to validate the parts that are passed in. Depending on how variable the input can be, I would go for this solution. But in other cases it’s just myself using my query construction logic in new ways. So I prefer a cheaper solution that makes me aware of the problem.

Thankfully, SQLAlchemy’s events allow us to be notified when a query is about to be executed. We can create a listener that raises an exception when we try to run a problematic query.

def before_execute(conn, clauseelement, multiparams, params):
    if (
        isinstance(clauseelement, sa.sql.selectable.Select)
        and len(clauseelement.froms) > 1
    ):
        raise RuntimeError("Cross join detected:\n{}".format(clauseelement))

sa.event.listen(engine, "before_execute", before_execute)

It’s not perfect because it does not check subqueries or CTEs but it gives us a line of defense. I am also thinking of adding an assertion that checks for the problem to the tests that exercise the query construction logic.

Here is the complete example code: sqlalchemy_implicit_cross_join.py.

PostgreSQL: JSON aggregation

Martin Vielsmaier — Fri, 16 Mar 2018 23:00:00 +0000

On a client project I have to do a search in a structure of tables and return results including some child objects. A classical instance of the N+1 query problem, if I was using an ORM. I decided not to use the ORM for the feature because it will be one of the hottest paths of the application and I wanted more control over the queries (rather complex logic with LIKE & sorting). But the filtering/sorting is not the topic today, so I will leave it out in the examples.

For illustration, let’s assume the following schema:

CREATE TABLE parents (
  parent_id INTEGER PRIMARY KEY,
  name VARCHAR
);

CREATE TABLE children (
  child_id INTEGER PRIMARY KEY,
  name VARCHAR,
  birthdate DATE,
  parent_id INTEGER,
  FOREIGN KEY (parent_id) REFERENCES parents
);

Without any further thinking I would do a query like this and a bit of code that constructs parent objects with their respective children.

SELECT * FROM parents JOIN children USING (parent_id) ORDER BY parent_id;

parent_id | name | child_id | name | birthdate  
-----------+-------+----------+---------+------------
         1 | Jim | 1 | Tamara | 2017-02-01
         1 | Jim | 3 | Tom | 2005-10-01
         1 | Jim | 5 | Tonja | 2011-07-17
         2 | Jenny | 2 | Tim | 2000-11-02
         2 | Jenny | 4 | Theresa | 2017-04-30

Just before I started writing the logic to pull apart the result and put it into it’s object structure, I thought, “It would be nice to let the database put together all the children of one parent into an array.”

I already knewarray_aggwhich aggregates all values into an array. After some reading I discoveredjson_build_objectwhich takes a sequence of keys and values and creates a JSON object from it.

So my final query looked like this:

SELECT
    parent_id
  , p.name
  , array_agg(json_build_object(
      'child_id', c.child_id
    , 'name', c.name
    , 'birthdate', c.birthdate
  )) as children
FROM parents p
JOIN children c USING (parent_id)
GROUP BY 1, 2;

parent_id | name | children                                                                                                              
-----------+-------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
         1 | Jim | {"{\"child_id\" : 1, \"name\" : \"Tamara\", \"birthdate\" : \"2017-02-01\"}","{\"child_id\" : 3, \"name\" : \"Tom\", \"birthdate\" : \"2005-10-01\"}","{\"child_id\" : 5, \"name\" : \"Tonja\", \"birthdate\" : \"2011-07-17\"}"}
         2 | Jenny | {"{\"child_id\" : 2, \"name\" : \"Tim\", \"birthdate\" : \"2000-11-02\"}","{\"child_id\" : 4, \"name\" : \"Theresa\", \"birthdate\" : \"2017-04-30\"}"}

When the query is executed with sqlalchemy, children in the resulting rows is already correctly typed as a list of dictionaries.

The explain analyze output for both queries (on a trivially small test set) shows that the aggregated version is 50-100% slower (150-200μs vs. 250-350μs), but I guess that will rarely be a real problem because - at least in my case

the execution time of my query is dominated by filtering/sorting the parent rows.

If you would like to play with the example yourself, get the SQL file here.

Python: bare except statement - why it is bad.

Martin Vielsmaier — Tue, 19 Sep 2017 22:00:00 +0000

The bare except statement is something I often see in the main loop of a script that should run indefinitely. Pylint will warn you about this problem, but in my opinion we should start to treat this as an error.

Consider the following code snippet:

import sys
import time

def risky_business():
  time.sleep(1)

while(True):
  try:
    risky_business()
  except:
    print("Problems?", sys.exc_info())

This will work nicely to keep your program running whatever happends inside yourisky_business function. But let’s try to stop your program by pressing CTRL-C.

$ python a.py 
^C('Problems?', (<type 'exceptions.KeyboardInterrupt'>, KeyboardInterrupt(), <traceback object at 0x7f234f6c9dd0>))

We caught the KeyboardInterrupt exception and just carried on. But we catch more which we should not. Let’s try to exit the program from the business function, maybe because the user asked for it or we got signaled.

def risky_business():
  time.sleep(1)
  sys.exit(0)

$ python a.py 
('Problems?', (<type 'exceptions.SystemExit'>, SystemExit(0,), <traceback object at 0x7f49b7ab5e18>))
('Problems?', (<type 'exceptions.SystemExit'>, SystemExit(0,), <traceback object at 0x7f49b7ab5ea8>))
...

So why is this happening? Both KeyboardInterrupt and SystemExit are exceptions (derived from BaseException). An except statement without any restriction will catch any exception derived from BaseException. Here is the complete hierachy of the builtin exceptions.

We can fix our problem by restricting the kind of exception that we want to catch:

def risky_business():
  time.sleep(1)

while(True):
  try:
    risky_business()
  except Exception:
    print("Problems?", sys.exc_info())

When we press CTRL-C now, the program exits with the right exception.

$ python a.py
^CTraceback (most recent call last):
  File "a.py", line 10, in <module>
    risky_business()
  File "a.py", line 6, in risky_business
    time.sleep(1)
KeyboardInterrupt

Pylint will still warn you about this line, because catching Exception is still very broad and you should only do this when you have a good reason, e.g. the reason mentioned above (to keep a service running even if it could not handle a certain input or request).

So next time you see a bare except statement in your code, take a moment to reconsider if you really want to catch SystemExit, KeyboardInterrupt (andGeneratorExit which I did not mention above). If that is the case, you might want to make it explicit so that the next person reading the code does not have to check again.