Making QuerySets composable

2016-01-22 Thread Patryk Zawadzki
Hi,

Currently the way QuerySets work makes it hard to reuse code. Consider an
example where I have template tags to render an author and a book (that in
turn displays author). I don't want my template code to execute any
additional queries per row displayed so I am forced to prefetch everything
upfront. Now if I want to display a list of authors and a list of books my
code is vastly different:

```
authors = Author.objects.filter(visible=True)
authors_for_display = authors.annotate(num_books=Count('books'),
num_awards=Count('awards')).prefetch_related('foo')
```

vs.

```
books = Book.objects.filter(visible=True, author__visible=True)
books_for_display = books.annotate(author_num_books=Count('author__books'),
author_num_awards=Count('author__awards')).select_related('author').prefetch_related('author__foo')
```

Both cases need to do the exact same filtering, annotations and prefetches
for the Author object but notation is so different that no code reuse can
happen ("baz" vs. "bar__baz" vs. "foo__bar__baz"). The behaviour of
annotations is also different as now they end up on the Book object which
forces me to write some glue code. If my structure gains another level of
depth I will be forced to go to even greater extents.

Now consider that I have many other objects that reference the Author and
thus I have many places in code that implement the same logic but each time
with a slight syntactic twist. What if a front-end engineer asks me to
change the information fetched about the Author? Or perhaps I discover that
half of the query time is spent fetching a megabyte of Author's extended
biography in HTML that the front-end never uses outside of that Author's
page. I am forced to hunt down and update all of these.

Currently we have Prefetch objects that kind of solve this problem for M2M
relations but perhaps it would be nice to also be able to use QuerySets in
select_related() or even in filter(). I don't think Prefetch objects are
best suited for that and I personally find having to instantiate them
explicitly kind of ugly. To me having another SelectRelated object is
probably a no-go.

Ideas?

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CANw2pUHwXq_9sVCPaK5CFXLzowd8HmemxVBe4%2BU7NJZ6QAccOw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: Making QuerySets composable

2016-01-22 Thread charettes
Hi Patryk,

> Currently we have Prefetch objects that kind of solve this problem for M2M
> relations but perhaps it would be nice to also be able to use QuerySets in
> select_related() or even in filter(). I don't think Prefetch objects are 
best
> suited for that and I personally find having to instantiate them 
explicitly
> kind of ugly.

>From what I understand you're not a fan of Prefetch objects but I think most
of your problems can be solved by using them and custom managers as the 
formers
also support foreign keys[1].

class AuthorDisplayManager(models.Manager):
def get_queryset(self):
queryset = super().get_queryset()
return queryset.filter(
visible=True,
).annotate(
num_books=Count('books'),
num_awards=Count('awards'),
).prefetch_related(
'foo',
)

class Author(models.Model):
...
for_display = AuthorDisplayManager()


class BookDisplayManager(models.Manager):
def get_queryset(self):
queryset = super().get_queryset()
return queryset.filter(
visible=True,
).prefetch_related(
Prefetch(
'author',
Author.for_display.all(),
)
)


class Book(models.Model):
...
for_display = BookDisplayManager()

Simon

[1] https://code.djangoproject.com/ticket/17003

Le vendredi 22 janvier 2016 05:25:24 UTC-5, Patryk Zawadzki a écrit :
>
> Hi,
>
> Currently the way QuerySets work makes it hard to reuse code. Consider an 
> example where I have template tags to render an author and a book (that in 
> turn displays author). I don't want my template code to execute any 
> additional queries per row displayed so I am forced to prefetch everything 
> upfront. Now if I want to display a list of authors and a list of books my 
> code is vastly different:
>
> ```
> authors = Author.objects.filter(visible=True)
> authors_for_display = authors.annotate(num_books=Count('books'), 
> num_awards=Count('awards')).prefetch_related('foo')
> ```
>
> vs.
>
> ```
> books = Book.objects.filter(visible=True, author__visible=True)
> books_for_display = 
> books.annotate(author_num_books=Count('author__books'), 
> author_num_awards=Count('author__awards')).select_related('author').prefetch_related('author__foo')
> ```
>
> Both cases need to do the exact same filtering, annotations and prefetches 
> for the Author object but notation is so different that no code reuse can 
> happen ("baz" vs. "bar__baz" vs. "foo__bar__baz"). The behaviour of 
> annotations is also different as now they end up on the Book object which 
> forces me to write some glue code. If my structure gains another level of 
> depth I will be forced to go to even greater extents.
>
> Now consider that I have many other objects that reference the Author and 
> thus I have many places in code that implement the same logic but each time 
> with a slight syntactic twist. What if a front-end engineer asks me to 
> change the information fetched about the Author? Or perhaps I discover that 
> half of the query time is spent fetching a megabyte of Author's extended 
> biography in HTML that the front-end never uses outside of that Author's 
> page. I am forced to hunt down and update all of these.
>
> Currently we have Prefetch objects that kind of solve this problem for M2M 
> relations but perhaps it would be nice to also be able to use QuerySets in 
> select_related() or even in filter(). I don't think Prefetch objects are 
> best suited for that and I personally find having to instantiate them 
> explicitly kind of ugly. To me having another SelectRelated object is 
> probably a no-go.
>
> Ideas?
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/ebad6483-f90c-41f9-8710-65b328320cdd%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Making QuerySets composable

2016-01-22 Thread Patryk Zawadzki
pt., 22.01.2016 o 17:44 użytkownik charettes  napisał:

> Hi Patryk,
>
>
> > Currently we have Prefetch objects that kind of solve this problem for
> M2M
> > relations but perhaps it would be nice to also be able to use QuerySets
> in
> > select_related() or even in filter(). I don't think Prefetch objects are
> best
> > suited for that and I personally find having to instantiate them
> explicitly
> > kind of ugly.
>
> From what I understand you're not a fan of Prefetch objects but I think
> most
> of your problems can be solved by using them and custom managers as the
> formers
> also support foreign keys[1].
>

Funny, I was aware of the featurebut always assumed prefetch_related would
always execute an extra query instead of becoming an inner join.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CANw2pUHOmpeq%3Db4jbdQG%3D8qt_p_tTjDouaPMPe8AD3VnyMoYLw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Query on BooleanField with values other than True and False, bug or intentional?

2016-01-22 Thread Kaveh Karimi
Today I discovered I can use strings and numbers to query on BooleanFields.

Let's say we have the following model:

class Whatever(models.Model):
name = models.CharField(max_length=200)
is_active = models.BooleanField()

The following queries return all instances which their `is_active` field is 
True:

Whatever.object.filter(is_active=True)
Whatever.object.filter(is_active='some_random_text')
Whatever.object.filter(is_active=777)

and the followings return the ones which are False:

Whatever.object.filter(is_active=False)
Whatever.object.filter(is_active='')
Whatever.object.filter(is_active=0)

Is this behaviour intentional or is it a bug? Should queries on 
BooleanFields accept strings and numbers?

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/3589aed0-0adf-41e5-8098-7e474c5d18ba%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Query on BooleanField with values other than True and False, bug or intentional?

2016-01-22 Thread Tim Graham
It doesn't seem to me that changing the behavior at this point is worth 
possible backwards incompatibilities.

On Friday, January 22, 2016 at 2:51:41 PM UTC-5, Kaveh wrote:
>
> Today I discovered I can use strings and numbers to query on BooleanFields.
>
> Let's say we have the following model:
>
> class Whatever(models.Model):
> name = models.CharField(max_length=200)
> is_active = models.BooleanField()
>
> The following queries return all instances which their `is_active` field 
> is True:
>
> Whatever.object.filter(is_active=True)
> Whatever.object.filter(is_active='some_random_text')
> Whatever.object.filter(is_active=777)
>
> and the followings return the ones which are False:
>
> Whatever.object.filter(is_active=False)
> Whatever.object.filter(is_active='')
> Whatever.object.filter(is_active=0)
>
> Is this behaviour intentional or is it a bug? Should queries on 
> BooleanFields accept strings and numbers?
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/5d4ff4a5-17c6-4bc9-9a11-14a1d2bf0650%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: SchemaEditor: Alter Default requires column definition

2016-01-22 Thread Tim Graham
What sort of comments are you looking for?

On Sunday, January 17, 2016 at 12:47:00 PM UTC-5, Max Bothe wrote:
>
> Hi all,
>
> Some databases (like SAP HANA) don't support adding or removing a default 
> value of a column without redefining the column [1].
> In order to support custom backends for these databases, the column 
> definition has to be passed additionally to the format strings 
> sql_alter_column_default and sql_alter_column_no_default in 
> django.db.backend.base.schema.BaseSchemaEditor .
>
> Comments are welcome.
>
> Thanks,
> Max  
>
>
> [1] 
> https://help.sap.com/saphelp_hanaplatform/helpdata/en/20/d329a6751910149d5fdbc4800f92ff/content.htm#loio20d329a6751910149d5fdbc4800f92ff__sql_alter_table_1alter_table_alter_column_clause
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/8a24f35c-cae0-4045-903f-1428ecbdd5a9%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [Review Request] Added support for database delegated fields (like AutoField)

2016-01-22 Thread Tim Graham
It would be helpful if you could sketch out the proposal a bit more -- 
maybe write some documentation that explains and motivates the feature (and 
put explanation in this thread). The pull request description has some 
explanation but it doesn't seem suited toward someone unfamiliar with the 
background of the ticket which could explain the lack of response here.

On Wednesday, January 6, 2016 at 10:04:51 AM UTC-5, Owais Lone wrote:
>
> Hi, I would really appreciate if someone could review the API design (and 
> implementation) for a PR I created. The PR implements support for fields 
> that are assigned a value by the database (using a default value or 
> trigger) instead of Django. It also adds support for PostgreSQL’s RETURNING 
> and Oracle’s RETURNING INTO clauses. 
>
> This will allow us to have AutoField like behaviour on fields other than 
> the primary key or have primary key fields other than Integer and 
> BigInteger types. 
>
> The PR also sets the stage for returning primary keys (and optionally 
> other fields) from the bulk_create operation on PostgreSQL and Oracle. 
>
> Pull request: https://github.com/django/django/pull/5904 
> Django ticket: https://code.djangoproject.com/ticket/21454 
>
> Thanks

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/9612ab62-df34-438f-b475-1d96eb52ecb6%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Query on BooleanField with values other than True and False, bug or intentional?

2016-01-22 Thread Maxime Lorant
At least, the behaviour is predictable : it uses `bool(value)`. I believe 
it is not a bug but more a question about the input definition: should we 
allow non boolean value? I don't see any problem here accepting a string as 
a `True` value, it is the job of the user to ensure the value is castable 
to a boolean.

The same behaviour is found on `IntegerField` for example: 
`Model.objects.filter(pk="foo")` raises `ValueError: invalid literal for 
int() with base 10: 'foo'` which implies the ORM tried to cast 'foo' as an 
integer.

On Friday, January 22, 2016 at 8:51:41 PM UTC+1, Kaveh wrote:
>
> Today I discovered I can use strings and numbers to query on BooleanFields.
>
> Let's say we have the following model:
>
> class Whatever(models.Model):
> name = models.CharField(max_length=200)
> is_active = models.BooleanField()
>
> The following queries return all instances which their `is_active` field 
> is True:
>
> Whatever.object.filter(is_active=True)
> Whatever.object.filter(is_active='some_random_text')
> Whatever.object.filter(is_active=777)
>
> and the followings return the ones which are False:
>
> Whatever.object.filter(is_active=False)
> Whatever.object.filter(is_active='')
> Whatever.object.filter(is_active=0)
>
> Is this behaviour intentional or is it a bug? Should queries on 
> BooleanFields accept strings and numbers?
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/7aa8cb6f-b747-4b2d-ba22-4d62ce3f1fa0%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Refactor year, month, day lookups?

2016-01-22 Thread Paulo Maciel
+1

Em quarta-feira, 25 de março de 2015 01:24:44 UTC-3, Josh Smeaton escreveu:
>
> Hi,
>
> Firstly (and least importantly) opening a PR makes it a lot easier to 
> review code, especially when there are lots of commits. A [WIP] pull 
> request is common and useful. If you get a chance, you should open one with 
> this change.
>
> I think it's a good idea. So much so that I opened a ticket about a year 
> ago: https://code.djangoproject.com/ticket/22394. You'll note some 
> comments there about retaining the Year based behaviour as a `BETWEEN X and 
> Y` rather than `Extract(YEAR)`. Otherwise, I think the support is rather 
> positive. At a high level, your code looks fairly solid and I think would 
> be a useful addition.
>
> Another thing I would really like to see is transform based annotations. 
> I'm not 100% sure on whether .annotate(F('X__transform')) is supported or 
> not, but if it is, we'll get some really nice behaviour from the use of 
> transforms.
>
> Think:
>
> sales_per_month = Model.objects.annotate(month=F('mydate__month')).values(
> 'month').Aggregate(sales=Sum('sale'))
>
> If Transforms don't yet work with annotate, that'll probably be what I'd 
> like to implement next. But the first step is having transforms to work 
> with, where date based transforms are (to me) the most useful.
>
> Cheers,
>
> On Wednesday, 25 March 2015 13:39:39 UTC+11, Jon Dufresne wrote:
>>
>> Hi, 
>>
>> I have been spending some time learning and investigating the custom 
>> lookups feature that was newly introduced in 1.7 [0]. While 
>> investigating, I wanted to learn by example so I started looking 
>> through the Django code. In the end, I didn't find many examples. 
>> However, I did notice that there exists lookups for the year, month, 
>> day, (and more) value from a database date or datetime value [1]. 
>> These appear to be implemented as special case "builtin" lookups and 
>> not through the new lookup mechanism. Is there a specific reason for 
>> this or is it historical momentum? 
>>
>> I started investigating if these could be refactored to use the common 
>> code path and implemented using the new lookup mechanism. To my 
>> delight it was not very difficult and I now have all tests passing 
>> after refactoring these lookups. Right now, this lives in a branch of 
>> mine and not in a ticket or pull request. The WIP branch is located 
>> at:  
>>
>> Would this be something welcome as a ticket and pull request? While 
>> there is no outward change in functionality, I see it as a beneficial 
>> refactoring because with this change: 
>>
>> 1. The year, month, day, etc lookups are no longer treated as special 
>> cases, but instead use the common code path. 
>> 2. There now exists complete and useful examples of registering new 
>> lookups in the Django code itself. This might help others build more 
>> lookups. 
>> 3. The lookups are now limited to the correct Field types where as 
>> previously this was not true. I demonstrate this with a unit test. 
>>
>> If this looks like it could be a welcome change I will can go forward 
>> with a typical ticket and pull request. 
>>
>> Cheers, 
>> Jon 
>>
>> [0] https://docs.djangoproject.com/en/dev/howto/custom-lookups/ 
>> [1] https://docs.djangoproject.com/en/dev/ref/models/querysets/#year 
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/d85a3625-c0c6-4001-8c4e-fea17715cedf%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Query on BooleanField with values other than True and False, bug or intentional?

2016-01-22 Thread sbrandt
I agree on the "it is the job of the user" thing as well as on the 
predictability on IntegerField, but there is at least one case where this 
goes horribly wrong. Imagine in the web-world we are receiving JSON and for 
some case, the browser sends "false" as a string. `bool('false') -> True`.

One could say this is again a mistake of the app developer, but my 
understanding of "explicit is better than implicit" is that Django should 
explicitly protect me from those mistakes that could stay undetected for a 
*very* long time because it just happily silently implicitly converts data 
types like PHP.

Maybe a backwards compatible solution here would be to provide an 
`strict=False` parameter to model fields or provide `StrictIntegerField` 
etc subclasses, although this may end up in a mess.

Am Freitag, 22. Januar 2016 22:18:45 UTC+1 schrieb Maxime Lorant:
>
> At least, the behaviour is predictable : it uses `bool(value)`. I believe 
> it is not a bug but more a question about the input definition: should we 
> allow non boolean value? I don't see any problem here accepting a string as 
> a `True` value, it is the job of the user to ensure the value is castable 
> to a boolean.
>
> The same behaviour is found on `IntegerField` for example: 
> `Model.objects.filter(pk="foo")` raises `ValueError: invalid literal for 
> int() with base 10: 'foo'` which implies the ORM tried to cast 'foo' as an 
> integer.
>
> On Friday, January 22, 2016 at 8:51:41 PM UTC+1, Kaveh wrote:
>>
>> Today I discovered I can use strings and numbers to query on 
>> BooleanFields.
>>
>> Let's say we have the following model:
>>
>> class Whatever(models.Model):
>> name = models.CharField(max_length=200)
>> is_active = models.BooleanField()
>>
>> The following queries return all instances which their `is_active` field 
>> is True:
>>
>> Whatever.object.filter(is_active=True)
>> Whatever.object.filter(is_active='some_random_text')
>> Whatever.object.filter(is_active=777)
>>
>> and the followings return the ones which are False:
>>
>> Whatever.object.filter(is_active=False)
>> Whatever.object.filter(is_active='')
>> Whatever.object.filter(is_active=0)
>>
>> Is this behaviour intentional or is it a bug? Should queries on 
>> BooleanFields accept strings and numbers?
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/8d9458df-72ca-4e70-9d22-110c8473d61d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.