Specifying a join object in relation's primaryjoin leads to errors

[Former user]

If I create a join object and then use it as a parameter for relation's primaryjoin, the mapper will compile without complaining, but accessing the property that maps to such a relation generates illegal SQL and throws ProgrammingError.

Ie.:

class_mapper(Class).add_property('property', relation(OtherClass, primaryjoin=Class.other_id==OtherClass.id)))

works fine but:

myjoin=join(Class, OtherCalss, Class.other_id==OtherClass.id)
class_mapper(Class).add_property('property', relation(OtherClass, primaryjoin=myjoin))

Compiles but throws ProgrammingError on accessing Class.property.

If a join cannot be used to specify primaryjoin then the mapper should complain right away; otherwise, SQLA should somehow extract the join clause from the join when it's used in this way.

This issue may be related to changeset 5325.

I've attached a test case that's maybe a little more complicated than necessary but demonstrates the issue clearly.

[Former user]

Self-contained test case

[Mike Bayer]

I think the SQL error is exactly appropriate. You can put whatever expression you want inside of primaryjoin - you're telling SQLalchemy what SQL you'd like to render. Its up to the database to tell you the SQL itself is wrong, SQLA doesn't go through the vast complexity of duplicating the database's SQL parser.

[Former user]

I see. The confusion stems from the fact that parameter name ''primaryjoin'' suggests quite strongly that a join object can be passed, and in fact the mapper compiles happily without warning. Afterwards, when the property is accessed one may think that he has constructed the join object incorrectly, and focus on that. I know that I lost two days this way until I finally figured out that you can't pass a join for primaryjoin.

Maybe a line should be added in the documentation alerting to this fact. Ie. adding this to sqlalchemy.orm.relation documentation, section on primaryjoin:

Note that SQLAlchemy will not check that the ClauseElement makes sense at mapper compilation; ie. a Join object is a ClauseElement and can be legaly set for a primaryjoin but will lead to nonsensical SQL and exception upon accessing the property it's defined for.

[Mike Bayer]

Replying to guest:

I see. The confusion stems from the fact that parameter name ''primaryjoin'' suggests quite strongly that a join object can be passed, and in fact the mapper compiles happily without warning. Afterwards, when the property is accessed one may think that he has constructed the join object incorrectly, and focus on that. I know that I lost two days this way until I finally figured out that you can't pass a join for primaryjoin.

"primaryjoin" has many simple examples in the documentation. You're saying you spent two whole days on something and you never went to read the documentation on the flag, which illustrates exactly what a usual usage is ? I will grant that a more descriptive name for the flag is "primary_join_binary_condition" but that's a lot of typing.

[Mike Bayer]

I upgraded the check that primaryjoin/secondaryjoin are clause expressions to be column-expressions, suitable for a WHERE clause, in 7fedc5e0708c27b646d32f88f532b34f0f898997.

You will now get this error message:

ArgumentError: Argument 'primaryjoin' is expected to be of type '<class 'sqlalchemy.sql.expression.ColumnElement'>', got '<class 'sqlalchemy.sql.expression.Join'>'

[Former user]

Replying to zzzeek: I was trying to implement a very complicated relation, and figured the easier way is to try to create a join that gives me the correct result, then plug that join in the mapper. Upon getting the error messages I kept trying to fix the join. That's why it took so long :-)

In all fairness, the fact that there are no examples of using join for a primaryjoin in the documentation doesn't conclusively mean that it's wrong. In fact, documentation for relation states that primaryjoin is a ClauseElement, and join is a ClauseElement.

I think the fix you made is exactly appropriate.

[Mike Bayer]

docs updated in 72859dd13b5ade9f9204ef1c37aafcbf9655085b

[Mike Bayer]

Removing milestone: 0.6.0 (automated comment)