CTE alias forgotten if used in two different CTEs

[sqlalchemy-bot]

Migrated issue, originally created by Frazer McLean (@RazerM)

If we have a CTE named cte1 aliased as aa, the generated SQL is wrong when the alias is used in CTEs cte2 and cte3.

from sqlalchemy import Column, Integer, MetaData, Table, Text, select

meta = MetaData()
a = Table('a', meta, Column('id', Integer))
b = Table('b', meta, Column('id', Integer), Column('fid', Integer))
c = Table('c', meta, Column('id', Integer), Column('fid', Integer))

cte1 = (
    select([a.c.id])
    .cte(name='cte1')
)

aa = cte1.alias('aa')

cte2 = (
    select([b.c.id])
    .select_from(b.join(aa, b.c.fid == aa.c.id))
    .cte(name='cte2')
)

cte3 = (
    select([c.c.id])
    .select_from(c.join(aa, c.c.fid == aa.c.id))
    .cte(name='cte3')
)

stmt = (
    select([cte3.c.id, cte2.c.id])
    .select_from(cte2.join(cte3, cte2.c.id == cte3.c.id))
)
print(stmt)

Output (formatted)

WITH cte1 AS (
  SELECT a.id AS id 
  FROM a
), cte2 AS (
  SELECT b.id AS id
  FROM b
  JOIN cte1 AS aa ON b.fid = aa.id
), cte3 AS (
  SELECT c.id AS id 
  FROM c
  JOIN aa ON c.fid = aa.id
)
SELECT cte3.id, cte2.id 
FROM cte2 JOIN cte3 ON cte2.id = cte3.id

The problem is that cte3 has JOIN aa instead of JOIN cte1 AS aa.

I initially came across this using the PostgreSQL dialect (with psycopg2), but the dialect-independent test case above also demonstrates the problem.

It may be that using a CTE alias like this isn't supported but if cte1 was aliased as an existing table name, cte3 would be joining to that table instead of cte1.

I'm using SQLAlchemy v1.2.4.

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

fixing this I found myself inserting a large block of code in a place that is frequently encountered by existing tests. it seemed likely that the code to render the new "x AS y" was going to conflict with some other assumptions. But it looks like in all those cases the tests already hit, we are using a cte() and not a cte().alias(). That is, if I take all those tests and make a new one with alias(), it fails to write the correct SQL in just the same way as here.

TL;DR you caught a big fish

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

and a chunk of those tests are all called "test_recursive_union_no_alias_", and there's no "test_recursive_union_alias_" test :) even have the names ready.

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

https://gerrit.sqlalchemy.org/#/c/zzzeek/sqlalchemy/+/684

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

Check existing CTE for an alias name when rendering FROM clause

Fixed bug in CTE rendering where a :class:.CTE that was also turned into
an :class:.Alias would not render its "ctename AS aliasname" clause
appropriately if there were more than one reference to the CTE in a FROM
clause.

Change-Id: If8cff27a2f4faa5eceb59aa86398db6edb3b9e72
Fixes: #4204

→ 5f60dc6

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed status to closed

[sqlalchemy-bot]

Frazer McLean (@RazerM) wrote:

Thanks for the effort!

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

Check existing CTE for an alias name when rendering FROM clause

Fixed bug in CTE rendering where a :class:.CTE that was also turned into
an :class:.Alias would not render its "ctename AS aliasname" clause
appropriately if there were more than one reference to the CTE in a FROM
clause.

Change-Id: If8cff27a2f4faa5eceb59aa86398db6edb3b9e72
Fixes: #4204
(cherry picked from commit 5f60dc6)

→ 3cfa074

[sqlalchemy-bot]

Sebastian Bank (@xflr6) wrote:

Not sure if I was using it wrong, but doing .cte(recursive=True).alias('tree') like shown below worked before 1.2.5 (however, .cte('tree', recursive=True) still works):

import sqlalchemy as sa
import sqlalchemy.ext.declarative

engine = sa.create_engine('sqlite://')

class Node(sa.ext.declarative.declarative_base()):

    __tablename__ = 'node'

    id = sa.Column(sa.Integer, primary_key=True)
    parent_id = sa.Column(sa.ForeignKey('node.id'))

    @classmethod
    def tree(cls):
        child, parent = (sa.orm.aliased(cls, name=n) for n in ('child', 'parent'))
        tree_1 = sa.select([
                child.id.label('child_id'),
                child.parent_id.label('parent_id'),
            ]).where(child.parent_id != None)\
            .cte(recursive=True)\
            .alias('tree')
        tree_2 = sa.select([tree_1.c.child_id, parent.parent_id])\
            .select_from(tree_1.join(parent, parent.id == tree_1.c.parent_id))\
            .where(parent.parent_id != None)
        return tree_1.union_all(tree_2)

Node.metadata.create_all(engine)

tree = Node.tree()

query = sa.select([tree.c.child_id, tree.c.parent_id], bind=engine)

print(query)

query.execute()

Before:

WITH RECURSIVE tree(child_id, parent_id) AS 
(SELECT child.id AS child_id, child.parent_id AS parent_id 
FROM node AS child 
WHERE child.parent_id IS NOT NULL UNION ALL SELECT tree.child_id AS child_id, parent.parent_id AS parent_id 
FROM tree JOIN node AS parent ON parent.id = tree.parent_id 
WHERE parent.parent_id IS NOT NULL)
 SELECT tree.child_id, tree.parent_id 
FROM tree

After (no such table: anon_1):

WITH RECURSIVE tree(child_id, parent_id) AS 
(SELECT child.id AS child_id, child.parent_id AS parent_id 
FROM node AS child 
WHERE child.parent_id IS NOT NULL UNION ALL SELECT tree.child_id AS child_id, parent.parent_id AS parent_id 
FROM anon_1 AS tree JOIN node AS parent ON parent.id = tree.parent_id 
WHERE parent.parent_id IS NOT NULL)
 SELECT tree.child_id, tree.parent_id 
FROM tree

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

OK, so you are doing it differently than the documentation states at http://docs.sqlalchemy.org/en/latest/core/selectable.html?highlight=cte#sqlalchemy.sql.expression.Select.cte - the .union() is called against the CTE() object, not the alias(). Can you work with it that way for now?

[sqlalchemy-bot]

Sebastian Bank (@xflr6) wrote:

Thanks Mike (just to make sure: apart from doing .cte().alias('tree') instead of .cte('tree'), the query build-up is correct, right?).

I can adapt, just wanted to make sure you are informed. Maybe this can be noted in the migration notes,

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

I couldn't find any test that seemed to depend on the way it was previously so let me see if i can make it work again.

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

found it

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed status to reopened

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• added labels: sql

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• set milestone to "1.2.x"

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

alrighty

https://gerrit.sqlalchemy.org/#/q/Iaa63d65ad2b90c8693f9953fbb32dbb10c73a037

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

Track if we're rendering within the CTE recursively

Fixed a regression that occurred from the previous fix to 🎫4204 in
version 1.2.5, where a CTE that refers to itself after the
:meth:.CTE.alias method has been called would not refer to iself
correctly.

Change-Id: Iaa63d65ad2b90c8693f9953fbb32dbb10c73a037
Fixes: #4204

→ ef2859b

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

Track if we're rendering within the CTE recursively

Fixed a regression that occurred from the previous fix to 🎫4204 in
version 1.2.5, where a CTE that refers to itself after the
:meth:.CTE.alias method has been called would not refer to iself
correctly.

Change-Id: Iaa63d65ad2b90c8693f9953fbb32dbb10c73a037
Fixes: #4204
(cherry picked from commit ef2859b)

→ e05f91e

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed status to closed