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

Fix regression sql mode #965

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* Bugfix - Replace use of numpy aliases of built-in types with built-in type. (#938) PR #939
* Bugfix - `ExternalTable.delete` should not remove row on error (#953) PR #956
* Bugfix - Fix error handling of remove_object function in `s3.py` (#952) PR #955
* Bugfix - Fix sql code generation to comply with sql mode ONLY_FULL_GROUP_BY (#916) PR #965
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Bugfix - Fix sql code generation to comply with sql mode ONLY_FULL_GROUP_BY (#916) PR #965
* Bugfix - Fix sql code generation to comply with sql mode `ONLY_FULL_GROUP_BY` (#916) PR #965


### 0.13.2 -- May 7, 2021
* Update `setuptools_certificate` dependency to new name `otumat`
Expand Down
2 changes: 1 addition & 1 deletion datajoint/expression.py
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ def create(cls, arg, group, keep_all_rows=False):
if inspect.isclass(group) and issubclass(group, QueryExpression):
group = group() # instantiate if a class
assert isinstance(group, QueryExpression)
if keep_all_rows and len(group.support) > 1:
if keep_all_rows and len(group.support) > 1 or group.heading.new_attributes:
group = group.make_subquery() # subquery if left joining a join
join = arg.join(group, left=keep_all_rows) # reuse the join logic
result = cls()
Expand Down
1 change: 1 addition & 0 deletions docs-parts/intro/Releases_lang1.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* Bugfix - Replace use of numpy aliases of built-in types with built-in type. (#938) PR #939
* Bugfix - `ExternalTable.delete` should not remove row on error (#953) PR #956
* Bugfix - Fix error handling of remove_object function in `s3.py` (#952) PR #955
* Bugfix - Fix sql code generation to comply with sql mode ONLY_FULL_GROUP_BY (#916) PR #965
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Bugfix - Fix sql code generation to comply with sql mode ONLY_FULL_GROUP_BY (#916) PR #965
* Bugfix - Fix sql code generation to comply with sql mode ``ONLY_FULL_GROUP_BY`` (#916) PR #965


0.13.2 -- May 7, 2021
----------------------
Expand Down
4 changes: 1 addition & 3 deletions tests/test_aggr_regressions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@
"""

import itertools
from nose.tools import assert_equal, raises
from nose.tools import assert_equal
import datajoint as dj
from . import PREFIX, CONN_INFO

schema = dj.Schema(PREFIX + '_aggr_regress', connection=dj.conn(**CONN_INFO))

# --------------- ISSUE 386 -------------------
Expand Down Expand Up @@ -102,4 +101,3 @@ def test_issue558_part1():
def test_issue558_part2():
d = dict(id=3, id2=5)
assert_equal(len(X & d), len((X & d).proj(id2='3')))

6 changes: 6 additions & 0 deletions tests/test_groupby.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
from .schema_simple import A, D


def test_aggr_with_proj():
# issue #944 - only breaks with MariaDB
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jverswijver Not very clear on this one about the MariaDB breakage noted here. Let tag up on this too to understand more about this fix/test. Also, would be good to give an assert criteria of this test as opposed to simple expecting the query not to 'fail'.

A.aggr(D.proj(m='id_l'), ..., n='max(m) - min(m)')