Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alias as column expr needs tweak to self_group(), but don't know use case yet #3939

Closed
sqlalchemy-bot opened this issue Mar 16, 2017 · 14 comments
Labels
bug Something isn't working sql
Milestone

Comments

@sqlalchemy-bot
Copy link
Collaborator

Migrated issue, originally created by Michael Bayer (@zzzeek)

this produces nonsensical SQL, but putting Alias into a func is something they need for the Postgresql functions:

from sqlalchemy import *

t = table('t', column('x'))

expr = func.foobar(select([t]).alias())

stmt = select([expr])

print stmt

self_group() fails because it does not provide for "against" since it never expected to be called in a column context:

diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py
index b69d667..9db1e08 100644
--- a/lib/sqlalchemy/sql/selectable.py
+++ b/lib/sqlalchemy/sql/selectable.py
@@ -1241,13 +1241,13 @@ class Alias(FromClause):
                                                     or 'anon'))
         self.name = name
 
-    def self_group(self, target=None):
-        if isinstance(target, CompoundSelect) and \
+    def self_group(self, against=None):
+        if isinstance(against, CompoundSelect) and \
             isinstance(self.original, Select) and \
                 self.original._needs_parens_for_grouping():
             return FromGrouping(self)
 
-        return super(Alias, self).self_group(target)
+        return super(Alias, self).self_group(against=against)
 
     @property
     def description(self):
@@ -2269,7 +2269,7 @@ class CompoundSelect(GenerativeSelect):
                      n + 1, len(s.c._all_columns))
                 )
 
-            self.selects.append(s.self_group(self))
+            self.selects.append(s.self_group(against=self))
 
         GenerativeSelect.__init__(self, **kwargs)
 

still, the SQL from above is:

SELECT foobar(SELECT t.x 
FROM t) AS foobar_1 
FROM (SELECT t.x AS x 
FROM t) AS anon_1

so...I don't know yet what they are trying to make this do that makes any sense.

@sqlalchemy-bot
Copy link
Collaborator Author

Lukas Siemon (@lukas-gitl) wrote:

So I did some digging, and this might not be needed. However since we build our own serialisation/filter extension on top of SA, it wasn't trivial to fix the query generation logic and I patched it for now by using the following.

I would advice not to include this fix for now until someone comes up with an actual use case. Maybe put a warning when someone tries to use the Alias self_group with any kwargs?

import unittest
from sqlalchemy import Column, String, column, func, select, text
from sqlalchemy.dialects.postgresql import ARRAY, UUID
from sqlalchemy.ext.declarative import declarative_base
from flask import Flask
from flask_sqlalchemy import SQLAlchemy
from sqlalchemy.sql import Alias
from sqlalchemy.util import symbol

app = Flask(__name__)
app.config['SQLALCHEMY_DATABASE_URI'] = (
    'postgres://postgres:password@localhost:5432/gtmp')
app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = True
db = SQLAlchemy(app)

Base = declarative_base()


class Keyword(db.Model):
    __tablename__ = 'keyword'
    id = Column(UUID, primary_key=True, nullable=False,
                server_default=text('uuid_generate_v4()'))
    names = Column(ARRAY(String), nullable=False)

db.create_all()


# Patch alias self_group kwargs
def patched_alias_self_group(self, target=None, **kwargs):
    if kwargs.pop('against') == symbol('_asbool'):
        return original_alias_self_group(self, target=target).as_scalar()
    return original_alias_self_group(self, target=target)
original_alias_self_group = Alias.self_group
Alias.self_group = patched_alias_self_group


class TestClass(unittest.TestCase):

    def test_function(self):
        query = Keyword.query

        search_string = 'n'
        query = query.filter(
            select([
                func.bool_or(column('e').ilike('%' + search_string + '%'))
            ]).select_from(
                func.unnest(Keyword.names).alias('e')
            ).alias()
        )

        print query

        print query.all()

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

the method is inconsistent vs. all the rest and I'm sure we'll need it once I implement more of the PG functions, I'll put it in 1.2.

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

this isn't critical until we get to the Postgresql stuff

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • changed milestone from "1.1.x" to "1.3"

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

Use consistent method signature for Alias.self_group()

Fixed bug where the use of an :class:.Alias object in a column
context would raise an argument error when it tried to group itself
into a parenthesized expression. Using :class:.Alias in this way
is not yet a fully supported API, however it applies to some end-user
recipes and may have a more prominent role in support of some
future Postgresql features.

Change-Id: I81717e30416e0350f08d1e022c3d84656e0a9735
Fixes: #3939

7bb4923

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • changed status to closed

@sqlalchemy-bot
Copy link
Collaborator Author

Lukas Siemon (@lukas-gitl) wrote:

I finally found a proper use case for this:

import unittest
from sqlalchemy import (Column, func, select, Integer, event, and_, Table)
from sqlalchemy.ext.declarative import declarative_base
from flask import Flask
from flask_sqlalchemy import SQLAlchemy
from sqlalchemy.orm import mapper, aliased, relationship, joinedload
from sqlalchemy.sql import Alias

app = Flask(__name__)
app.config['SQLALCHEMY_DATABASE_URI'] = (
    'postgres://postgres:password@localhost:5432/tmp')
app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = True
db = SQLAlchemy(app)

Base = declarative_base()


class Venue(db.Model):
    __tablename__ = 'venue'
    id = Column(Integer, primary_key=True, nullable=False)


class Offer(db.Model):
    __tablename__ = 'offer'
    id = Column(Integer, primary_key=True, nullable=False)


offer_to_venue = Table(
    'offer_to_venue',
    db.metadata,
    Column('offer_id', Integer, primary_key=True, nullable=False),
    Column('venue_id', Integer, primary_key=True, nullable=False)
)


@event.listens_for(mapper, "mapper_configured")
def mapper_listener(mapper_, cls):
    if not issubclass(cls, Offer):
        return

    venue = aliased(Venue)
    offer_to_venue_alias = aliased(offer_to_venue)
    cls.venues_nearby = relationship(
        Venue, secondary=offer_to_venue,
        primaryjoin=cls.id == offer_to_venue.c.offer_id,
        secondaryjoin=and_(
            offer_to_venue.c.venue_id == Venue.id,
            Venue.id == func.any(select([
                venue.id
            ]).where(and_(
                # venue id linked to offer
                offer_to_venue.c.offer_id == offer_to_venue_alias.c.offer_id,
                offer_to_venue_alias.c.venue_id == venue.id
            )).order_by(
                # usually dynamically ordered by distance
                venue.id
            ).limit(6).alias("venues_nearby"))
        ),
        viewonly=True,
        uselist=True
    )

db.create_all()


# Patch alias self_group kwargs
def patched_alias_self_group(self, target=None, **kwargs):
    return original_alias_self_group(self, target=target)
original_alias_self_group = Alias.self_group
Alias.self_group = patched_alias_self_group


class TestClass(unittest.TestCase):

    def test_function(self):
        query = Offer.query
        query = query.options(joinedload(Offer.venues_nearby))
        print query

Generates SQL:

SELECT
   offer.id AS offer_id,
   venue_1.id AS venue_1_id 
FROM
   offer 
   LEFT OUTER JOIN
      (
         offer_to_venue AS offer_to_venue_1 
         JOIN
            venue AS venue_1 
            ON offer_to_venue_1.venue_id = venue_1.id 
            AND venue_1.id = any(
            SELECT
               venue_2.id 
            FROM
               venue AS venue_2, offer_to_venue AS offer_to_venue_2 
            WHERE
               offer_to_venue_1.offer_id = offer_to_venue_2.offer_id 
               AND offer_to_venue_2.venue_id = venue_2.id 
            ORDER BY
               venue_2.id LIMIT % (param_1)s)
      )
      ON offer.id = offer_to_venue_1.offer_id

@sqlalchemy-bot
Copy link
Collaborator Author

Lukas Siemon (@lukas-gitl) wrote:

@zzzeek Does this make sense to you or should this be written differently? Would this be the basis for a test case?

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

the thing that was fixed here is really small and the fix has a simple test already. As long as your test program above now works without that monkeypatch, there's nothing to do here.

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • changed milestone from "1.3" to "1.2"

@sqlalchemy-bot
Copy link
Collaborator Author

Lukas Siemon (@lukas-gitl) wrote:

Sounds good! Thanks for changing it to 1.2

I remember we were struggling to come up with a real world use case for this one. Would you agree that this is a proper use case or should the above be rewritten differently?

The general goal is to have a (viewonly) many-to-many relationship that only returns a subset of the objects.

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

it looks like you're using any(), which is in the realm of crazy PG functions where FROM clauses get stuck into functions. Which is legit.

does your test case work now that the issue here is resolved?

@sqlalchemy-bot
Copy link
Collaborator Author

Lukas Siemon (@lukas-gitl) wrote:

We are currently still on 1.1.9. I will make a task here to upgrade soon so I can test this. In 1.1.9 we still rely on the monkey patch.

@sqlalchemy-bot
Copy link
Collaborator Author

Lukas Siemon (@lukas-gitl) wrote:

We have switched to 1.2 recently and this works as expected now.

@sqlalchemy-bot sqlalchemy-bot added bug Something isn't working sql labels Nov 27, 2018
@sqlalchemy-bot sqlalchemy-bot added this to the 1.2 milestone Nov 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sql
Projects
None yet
Development

No branches or pull requests

1 participant