Fix bug with unions. #964
Fix bug with unions. #964
Conversation
|
Seems the test coverage api we use is experiencing a major outage, tests all pass locally. We can run the github actions when they are back up (outage information). |
There was a problem hiding this comment.
@jverswijver Nice work on this one man! 🚀
Only small suggestions included but overall fix looks great.
tests/test_relational_operand.py
Outdated
| x = set(zip(*q1.fetch('i', 'j'))) | ||
| y = set(zip(*q2.fetch('i', 'j'))) | ||
| assert_set_equal(x, y) |
There was a problem hiding this comment.
Based on the lessons I learned earlier from the recursive comparison within lists discussion, we can simplify this now to:
| x = set(zip(*q1.fetch('i', 'j'))) | |
| y = set(zip(*q2.fetch('i', 'j'))) | |
| assert_set_equal(x, y) | |
| assert q1.fetch() == q2.fetch() |
There was a problem hiding this comment.
This seems to break the test, I get this error when running it:
(1064, "You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'UNION (SELECT DISTINCT `i`,`j` FROM `djtest_relational`.`#i_j` WHERE((`j`=2) AND' at line 1")
The other changes work fine.
There was a problem hiding this comment.
@jverswijver Just tagged up on this point and looks like this test should have passed. Must still be something missing with this fix. Let me know once you've pinpointed the problem/solution. Thanks 🤝
Co-authored-by: Raphael Guzman <38401847+guzman-raphael@users.noreply.github.com>
tests/test_relational_operand.py
Outdated
| x = set(zip(*q1.fetch('i', 'j'))) | ||
| y = set(zip(*q2.fetch('i', 'j'))) | ||
| assert_set_equal(x, y) |
There was a problem hiding this comment.
@jverswijver Just tagged up on this point and looks like this test should have passed. Must still be something missing with this fix. Let me know once you've pinpointed the problem/solution. Thanks 🤝
unified string formatting
|
It seems mysql does not support nested unions in 5.7. In 8.0 the issue is semi fixed where it allows left side nested unions, but throws a specific error for unsupported right side nested unions. However, there is a workaround for both versions where wrapping the query and all sub queries in a |
|
|
||
| def where_clause(self): | ||
| return '' if not self.restriction else ' WHERE(%s)' % ')AND('.join( | ||
| return '' if not self.restriction else ' WHERE (%s)' % ')AND('.join( |
There was a problem hiding this comment.
we don't really need the extra space but no big deal
There was a problem hiding this comment.
@jverswijver @A-Baji 🚀 Excellent job guys and great collab! 🚀
It is too bad that MySQL has such nuances in working with unions but glad we were able to workaround them to achieve full union nesting support.
|
Hi there, I am using datajoint version 0.13.7, and so I think this should be fixed based on what I've seen online for the different versions, but I am still getting this error when I try to do a union on more than 2 tables. Any advice? Thank you so much! |
Added handling for when union gets a union object inside its make_sql(), this allows you to do multiple unions.
Took the test from @guzman-raphael comments on the issue linked.
fix #926