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

peewee can infer alias references where the inference is unambiguous #965

Closed
keredson opened this Issue Jun 6, 2016 · 5 comments

Comments

Projects
None yet
2 participants
@keredson
Contributor

keredson commented Jun 6, 2016

code like:

Post.alias().select().where(Post.title=='something').get()

gens:

SELECT "t1"."id", "t1"."title", "t1"."author_id" FROM "post" AS t1
WHERE ("t2"."title" = ?) LIMIT 1 OFFSET 0

which throws an exception because the conditional isn't using the alias as a reference.

i know the official way would be to pull out the alias as a variable and reference the column from that, but this gets cumbersome w/ many aliases. where it's safe and unambiguous what the user intended (ie that table is only being used once in the query), peewee should just make the connection for you.

again i have code locally i could pull into PR if this is generally agreed on.

@coleifer

This comment has been minimized.

Owner

coleifer commented Jun 7, 2016

Aliased models are frequently used with correlated subqueries, making the possibility of inferring the correct thing to do quite a bit more complicated than it might seem.

Philosophically, I'm opposed to adding any heuristics into peewee. Peewee is incredibly easy to understand because it does what you tell it to do. For people who are comfortable writing SQL this is a big deal because there are no surprises in store when you go to try out your query. You don't need to know a bunch of specialized APIs, because peewee provides the right simple tools that you can use them to build up more complex tools.

@coleifer coleifer closed this Jun 7, 2016

@coleifer

This comment has been minimized.

Owner

coleifer commented Jun 7, 2016

Take a look at this post I wrote to get a longer take on these ideas: http://charlesleifer.com/blog/shortcomings-in-the-django-orm-and-a-look-at-peewee-a-lightweight-alternative/

@keredson

This comment has been minimized.

Contributor

keredson commented Jun 7, 2016

yep, read that about a week and a half ago. one of the reasons i gave peewee a real hard look, and i very much agree w/ it. i did a lot of django work ~2006-2008 and know well how glitchy their ORM can be. (tho to be fair, then it was light years ahead of anything else, when everything was full on "don't worry about the database, just pretend it's a giant in memory object graph" hell.) then (when thrown back into an all-java world), i did internally for my company just what you did in 2.0. pure SQL constructs w/ an immutable builder API (all java).

anyway, not meaning to life story here.

Philosophically, I'm opposed to adding any heuristics into peewee.

agree that's a good philosophically.

btw, i found having dealing w/ the alias objects outside the scope of the query a significant source of user complaints. i tried a couple different techniques to ease the pain. this was one. another was having the aliases, but not having to be tied to a specific object instance. like:

TableA.ALL.join(TableB.as("x")).where(TableB.as("x").COLUMN_Y.eq("whatever"));

(the ALL for me is like your select())

note the first as("x") is returning effectively what you call a model alias, where the second is (after the column is referenced) returning a bound field. this way people weren't having to declare an external variable outside their query. (granted, this declaration was more of a PITA in java)

anyway... :)

@coleifer

This comment has been minimized.

Owner

coleifer commented Jun 7, 2016

Thanks for the input. Although the whole way peewee does model aliases currently is a bit of a lucky hack (see c118e43), I like using descriptive variable names for my table aliases. I've found it much easier to read and keep track of my queries when I can think of subsequent table references as distinct row sources. Nothing wrong with .as('alias'), but the repetition does create an equally easy avenue for one to forget to include the alias.

@keredson

This comment has been minimized.

Contributor

keredson commented Jun 7, 2016

hah. been there.

yeah, the references also worked as how yours do as well. you could also do:

Table tableb = TableB.as("x");
TableA.ALL.join(tableb).where(tableb.COLUMN_Y.eq("whatever"));

but it just was never used very much that way.

keredson added a commit to keredson/peewee that referenced this issue Jun 7, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment