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

Using sub-select as operand in expression applies a table alias to the sub-select #1801

Closed
iksteen opened this issue Dec 4, 2018 · 7 comments

Comments

@iksteen
Copy link

iksteen commented Dec 4, 2018

Consider the following (simplified) example where I want to divide a value by the result of a subquery (in the actual use case the points will decay based on how many teams solved a challenge correctly):

import logging
from peewee import *

logging.basicConfig(level=logging.DEBUG)
database = PostgresqlDatabase('scoreboard')

class Base(Model):
    class Meta:
        database = database

class Team(Base):
    name = TextField(unique=True)

class Challenge(Base):
    flag = TextField(unique=True)
    points = IntegerField()

decayed_score = Challenge.select(
    Challenge.points / Team.select(fn.count(Team.id) + 1)
)
for row in decayed_score:
    print(row)

Running this example results in the following query:

DEBUG:peewee:('SELECT ("t1"."points" / (SELECT (count("t2"."id") + %s) FROM "team" AS "t2") AS "t3") FROM "challenge" AS "t1"', [1])

The alias applied to the subquery (T3) results in the following error:

peewee.ProgrammingError: syntax error at or near "AS"
LINE 1: ...ELECT (count("t2"."id") + 1) FROM "team" AS "t2") AS "t3") F...
                                                             ^
@iksteen
Copy link
Author

iksteen commented Dec 4, 2018

Hmm, it seems that if I use CompoundSelectQuery (which isn't imported using from peewee import *), it does work.

Is that the intended way to solve the problem?

Edit: I guess not, because trying to use a correlated subquery will create a new alias for the original table,

@iksteen
Copy link
Author

iksteen commented Dec 5, 2018

Is there a situation possible where either side of an expression can be aliased? I personally can't think of any, and if that is a correct assumption, the in_op context variable introduced by the fix for #1716 could be extended to include all operations (and in_op could be renamed to in_expr).

@iksteen iksteen changed the title Using subquery as an operand applies a table alias to the sunquery Using sub-select as operand in expression applies a table alias to the sub-select Dec 5, 2018
@coleifer
Copy link
Owner

coleifer commented Dec 5, 2018

You might notice that some operands will appear to work when the right-hand-side is a query object, but others will not work if the query is on the left-hand-side. This is because queries are not treated as column-like objects (even though in your example you are using a query as a column-like value). Furthermore in some cases attempting to use a query on the left-hand-side will result in the generation of JOIN clauses, since table-like objects (such as queries) implement operator overloads for representing JOINs...e.g. query1 + query2 == query1 UNION ALL query2.

Its not quite clear to me that creating a correlated subquery is the issue, as I've added test-cases showing that its possible: e321a23

The problem seems to arise when attempting to use a correlated subquery in an expression... Could you achieve what you're attempting by just using a join and aggregate instead?

@coleifer
Copy link
Owner

coleifer commented Dec 5, 2018

Original:

decayed_score = Challenge.select(
    Challenge.points / Team.select(fn.count(Team.id) + 1)
)

Alternative:

query = Challenge.select(Challenge.points / fn.COUNT(Team.id) + 1).join(Team, JOIN.LEFT_OUTER).group_by(...)

@iksteen
Copy link
Author

iksteen commented Dec 12, 2018

Ah right, unions could have an alias.

And yes, in this small example the aggregation that would work.. but it wouldn't fit my monster query (which I'm too afraid and ashamed to show).

However, by you having me rethink my method, you put me on a path where I think I can rewrite that monster query using CTEs where I won't wouldn't run into this issue.

That makes the issue not invalid, but feel free to close it anyway.

coleifer added a commit that referenced this issue Dec 14, 2018
@coleifer
Copy link
Owner

Think this is now resolved: fb41bfc

@iksteen
Copy link
Author

iksteen commented Dec 14, 2018

👍 That's exactly the way I got it to work locally (well, minus the parentheses rework).

Between you and me, this is my query now (please don't hate me): https://plak.infrapuin.nl/0bnaddnw.py This new commit allows me to get rid of the team_count cte.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants