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

Conversation

jverswijver
Copy link
Contributor

@jverswijver jverswijver commented Sep 21, 2021

add test for #944
fix #916 and added test

@jverswijver jverswijver marked this pull request as ready for review September 23, 2021 20:49
CHANGELOG.md Outdated
@@ -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

@@ -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



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'.

@guzman-raphael guzman-raphael merged commit b74c492 into datajoint:master Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0.13.X regression: aggr error with newly enforced sql_mode
3 participants