search

kvesteri / sqlalchemy-continuum

Versioning extension for SQLAlchemy.
BSD 3-Clause "New" or "Revised" License
578 stars 127 forks source link

SAWarning: fix secondary join for association table(s) #306

Open indiVar0508 opened 2 years ago

indiVar0508 commented 2 years ago

Attempt to address Fix for #305 , to address warning for SELECT statement has a cartetian product

tested in local for pytest for SQLA >1.4,<1.4 with SQLITE

result with Fix in PR: SQLA > 1.4 SQLITE 659 passed, 99 skipped, 219 warnings SQLA < 1.4 sqlite 659 passed, 99 skipped, 219 warnings

Master: SQLA > 1.4 SQLITE 659 passed, 99 skipped, 220 warnings SQLA < 1.4 sqlite 659 passed, 99 skipped, 220 warnings

indiVar0508 commented 2 years ago

Hi @marksteward ,

can you have a look at this, if changes are ok ,can you merge it, this shall address warnings related SQLA>1.4 and might also close #263

indiVar0508 commented 2 years ago

Hi @marksteward ,

did you get a chance to review this?

marksteward commented 2 years ago

I either leave comments or merge tickets when I review them.

josecsotomorales commented 1 year ago

Hey folks, got hit by this issue, does the change here fix it? Happy to take this PR and keep working on it, thoughts?

indiVar0508 commented 1 year ago

Hey @josecsotomorales , Yeah the PR addresses the warning but not sure if this is right way to do it ( i am newbie to sqlalchemy ), you can continue with this PR if you want.

stv8 commented 3 months ago

I think we want to combine the primary and secondary joins, not one or the other.

See my latest comment in the related ticket.