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

Fixed #28749 -- Added subquery support for PostgreSQL’s ArrayField 'in' lookup #9300

Merged
merged 1 commit into from Nov 1, 2017

Conversation

mpasternak
Copy link
Contributor

@mpasternak mpasternak commented Oct 27, 2017

Copy link
Sponsor Member

@adamchainz adamchainz left a comment

Choose a reason for hiding this comment

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

Looks straightforward to me, but please fix for flake8

@adamchainz adamchainz changed the title Subquery support for PosgteSQL’s ArrayField’s „in” lookups Fixed #28749 -- Added subquery support for PostgreSQL’s ArrayField 'in' lookup Oct 30, 2017
Copy link
Sponsor Member

@adamchainz adamchainz left a comment

Choose a reason for hiding this comment

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

Please also add a release note to the 2.0.txt file

@mpasternak
Copy link
Contributor Author

Hi @adamchainz , updated as suggested. As I understand, CI checks are being triggered automatically, right?

@adamchainz
Copy link
Sponsor Member

Yes there might just be a queue though

@@ -163,6 +163,9 @@ Minor features
* :djadmin:`inspectdb` can now introspect ``JSONField`` and various
``RangeField``\s (``django.contrib.postgres`` must be in ``INSTALLED_APPS``).

* The ``in`` lookup for `:class:`~django.contrib.postgres.fields.array.ArrayField`
supports subqueries
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

The ` before :class: is invalid
Add a period/full stop
📝🚀

self.assertSequenceEqual(
NullableIntegerArrayModel.objects.filter(
field__in=IntegerArrayModel.objects.all().values_list(
"field", flat=True)
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

this is probably cleaner as one line

@mpasternak
Copy link
Contributor Author

mpasternak commented Oct 30, 2017 via email

@@ -163,6 +163,9 @@ Minor features
* :djadmin:`inspectdb` can now introspect ``JSONField`` and various
``RangeField``\s (``django.contrib.postgres`` must be in ``INSTALLED_APPS``).

* The ``in`` lookup for :class:`~django.contrib.postgres.fields.array.ArrayField`
supports subqueries
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Last thing I can see, you still need to add a period (.) at the end of the sentence 😉

@adamchainz
Copy link
Sponsor Member

Still a couple CI failures

doc:

/home/jenkins/workspace/docs/docs/releases/2.0.txt:166: WARNING: py:class reference target not found: django.contrib.postgres.fields.array.ArrayField

Looks like this needs fixing, should be simple

isort:

+++ /home/jenkins/workspace/isort/tests/postgres_tests/test_array.py:after	2017-10-30 11:40:04.624306
@@ -11,6 +11,7 @@
 from django.test import TransactionTestCase, modify_settings, override_settings
 from django.test.utils import isolate_apps
 from django.utils import timezone
+
 from . import PostgreSQLTestCase, PostgreSQLWidgetTestCase
 from .models import (

That newline you removed should be there 😉

@mpasternak
Copy link
Contributor Author

@adamchainz CI says thing's legit now, thank you for your suggestions.

Copy link
Sponsor Member

@adamchainz adamchainz left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants