Re: group-by promotion

2016-01-10 Thread Anssi Kääriäinen 
On Saturday, January 9, 2016 at 7:01:38 AM UTC+2, Curtis Maloney wrote:
>
> Hey all, 
>
> I'm trying to write a PR for adding Postgres9.4's "FILTER" syntax for 
> aggregates, but I've hit a bug I can't figure how to fix [and nobody's 
> awake on IRC :)] 
>
> https://github.com/django/django/pull/5956 
>
> Basically, for the query: 
>
>  qset = FilterAggParentModel.objects.annotate( 
>  zero_count=FilterAgg(Count('filteraggtestmodel'), 
> Q(filteraggtestmodel__integer_field=1)), 
>  child_count=Count('filteraggtestmodel'), 
>  ) 
>
> It generates the SQL: 
>
> SELECT "postgres_tests_filteraggparentmodel"."id", 
>  COUNT("postgres_tests_filteraggtestmodel"."id") FILTER(WHERE 
> "postgres_tests_filteraggtestmodel"."integer_field" = 0) AS "zero_count", 
>  COUNT("postgres_tests_filteraggtestmodel"."id") AS "child_count" 
> FROM "postgres_tests_filteraggparentmodel" 
> LEFT OUTER JOIN "postgres_tests_filteraggtestmodel" ON 
> ("postgres_tests_filteraggparentmodel"."id" = 
> "postgres_tests_filteraggtestmodel"."parent_id") 
> GROUP BY "postgres_tests_filteraggparentmodel"."id", 
> "postgres_tests_filteraggtestmodel"."integer_field" 
>
>
> The problem being that 
> "postgres_tests_filteraggtestmodel"."integer_field" is included in the 
> GROUP BY clause... 
>
> Does anyone who understands this part of the ORM have a suggestion for 
> how to fix this? 
>

If you don't return anything from get_group_by_cols() no part of the 
expression should be added to the GROUP BY clause. I wonder why the 
integer_field is added to the group by, a bug somewhere in compiler maybe?

 - Anssi

-- 
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/980bd68a-f75c-4d64-a18f-58420bc45859%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: PostGres 9.5 Upsert

2016-01-10 Thread Anssi Kääriäinen 
On Sun, Jan 10, 2016 at 5:22 PM, Sean Brant  wrote:

> I've always considered this a bug waiting to happen. I have seen many errors
> with the get operation failing because it returned more then one value.
> Usually you don't notice the error until you hit production. I always
> suggest the lookup use fields that have unique indexes.
>
> Changing that would be backwards incompatible so maybe it's a docs issue.

If there are no valid use cases for using get_or_create() without an
unique index, then we could consider doing a backwards incompatible
change here. I believe there are cases where you want to do a
get_or_create() without  unique index (though I can't come up with one
right now).

We could always check if there is a backing unique index and use
get_or_create only in those cases.

 - Anssi

-- 
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/CALMtK1EbwD7hJLFOjsiUeMxofov9zwRbX_Zr%2BZArqdy%2Bj_fT_g%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: PostGres 9.5 Upsert

2016-01-10 Thread Anssi Kääriäinen 
On Sun, Jan 10, 2016 at 11:10 AM, Florian Apolloner
 wrote:
> On Sunday, January 10, 2016 at 9:09:37 AM UTC+1, Anssi Kääriäinen wrote:
>>
>> The save() operation matches the semantics of upsert exactly - maybe we
>> could use upsert there?
>
> Not sure .save() would be a good candidate for that. In the best case,
> .save() should execute one INSERT or UPDATE query without extra junk for
> "error" handling. Ie if you set a pk and want to update then pass
> force_update into it. I know that we already try UPDATE followed by INSERT
> in save() but I am wondering how often that is actually used by people.

Yes, it is likely that save() wouldn't benefit much from using upsert.
Upsert is likely a bit slower than plain update, and we end up doing
an update in almost all cases anyways. It could be an useful option
for some use cases, but it likely isn't a good default.

 - Anssi

-- 
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/CALMtK1H4d%3DqEm--XrZuf310VXhGP_zt3q6pjQ0i-tV4quTYuBw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: re-thinking middleware

2016-01-10 Thread Raphaël Barrois 
On Fri, 8 Jan 2016 11:38:04 -0700
Carl Meyer  wrote:

> Thanks everyone for the reviews. I've pushed a new commit addressing
> most of the issues raised:
> https://github.com/django/deps/commit/62b0ee351727cb0e7ef41ba6dd2f3f7280a219de
> 
> More comments and replies (to various people in the thread) below:
> 
> On 01/08/2016 09:01 AM, Florian Apolloner wrote:
> > PR1 (allows changing the middlewares during the middleware run): 
> > https://github.com/django/django/pull/5591
> > PR2 (static middleware chain, seems simpler and probably the way to
> > go): https://github.com/django/django/pull/5949/
> 
> Yes, the latter is what Pyramid does and what I had in mind; in fact,
> its performance benefits are one of the justifications given in the
> DEP for why to use the factory approach rather than simple functions.
> I don't think "changing middlewares per request" is a use case that's
> worth the performance implications (and it's not a use case we
> currently support anyway).
> 
> Thanks for the super-quick turnaround on implementation!
> 
> On 01/08/2016 10:35 AM, Aymeric Augustin wrote:
> > The only (and minor) concern I have is about allowing function-based
> >  middleware factories to return None.
> > 
> > In the spirit of “there should be only one way to do it”, I would 
> > require raising MiddlewareNotUsed explicitly to disable a
> > middleware. A middleware factory that returns None would cause an 
> > ImproperlyConfigured exception. Otherwise middleware could be
> > skipped by mistake, if the developer forgets to return the
> > middleware from the factory. This is especially a concern for
> > production-only middleware that developers can’t run locally.
> [snip]
> 
> Yes, you and Ryan are absolutely right, allowing `None` was a mistake
> and I've removed it from the spec. For function-based middleware,
> there's also the option to simply return the ``get_response`` callable
> you were given, which has the same effect of "skipping yourself." That
> falls out naturally from the concept, so I don't see any reason to
> disallow it, but we don't need to mention it in the docs if we want to
> stick to "one way to do it."
> 
> On 01/08/2016 04:51 AM, Markus Holtermann wrote:
> > I would, however, include the exception handling in the examples 
> > provided in section "Specification" as that is an integral part of 
> > middlewares, too.
> 
> Good idea, I've made that change in the commit linked above.
> 
> > Nitpicking, I would also name the settings variable MIDDLEWARES 
> > (i.e.plural) as it is a list of middlewares, not just one.
> 
> Well, this is a matter of some debate :-) See e.g.
> https://github.com/rack/rack/issues/332, where a number of people
> fervently argue that "middleware" is a "mass noun" along the lines of
> e.g. "furniture", and that it is even incorrect to say "a middleware"
> (much like you would never say "a furniture"); instead we should
> always say "a middleware component."
> 
> I think those people are fighting a lost battle; the usage of
> "middleware" as singular is already well established in the Python
> world, even outside Django; people frequently talk about "a WSGI
> middleware."
> 
> That said, my ear still prefers "middleware" as both the singular and
> the plural (there are such words in English) and dislikes
> "middlewares." So I'd prefer to stick with MIDDLEWARE and haven't
> changed it in the spec. But if most people think MIDDLEWARES is
> better, I won't stand in the way.
> 
> We could also go with something like MIDDLEWARE_FACTORIES or
> MIDDLEWARE_PIPELINE and avoid the issue altogether :-)
> 
> Carl
> 



Hi,

I've got only one minor suggestion to the "backwards compatibility"
section of the DEP.

> It currently states that "If the ``MIDDLEWARE`` setting is provided
> [...], the old ``MIDDLEWARE_CLASSES`` setting will be ignored.

I suggest that, instead, this fails loudly if both ``MIDDLEWARE`` and
``MIDDLEWARE_CLASSES`` are set.
This would prevent projects from keeping the two versions around and
having to remember which one is currently in use.


Beyond that, I love the overall design of this upgrade :)

-- 
Raphaël

-- 
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/20160111005458.796853f0%40corvis.
For more options, visit https://groups.google.com/d/optout.

Re: slow migrations

2016-01-10 Thread Shai Berger 
On Sunday 10 January 2016 08:09:33 Carl Meyer wrote:
> On 01/09/2016 09:29 AM, Shai Berger wrote:
> > There are two kinds of data migrations, generally speaking.
> > 
> > [...]
> >
> > The other is migrations which *create* data -- fill in tables for
> > database- implemented-enums, for example. If you remove these, you are
> > going to break your tests (if you do this and haven't broken your tests,
> > your tests are missing -- I'd go as far as calling them broken).
> 
> I consider this second kind of data migration to be marginally smelly,
> and avoid it whenever possible.

It should be noted that this kind of migration is what we recommend officially
as a substitute for initial_data fixtures:

https://docs.djangoproject.com/en/1.8/howto/initial-data/#automatically-loading-initial-data-fixtures

Shai.

Re: slow migrations

2016-01-10 Thread René Fleschenberg 
Hi,

On Saturday 09 January 2016 18:29:32 Shai Berger wrote:
> The other is migrations which *create* data -- fill in tables for
> database- implemented-enums, for example. If you remove these, you are
> going to break your tests (if you do this and haven't broken your tests,
> your tests are missing -- I'd go as far as calling them broken).

I agree with Carl. I avoid Django migrations for this kind of data. 

Obviously, you don't get dependency handling that way, but I think this is 
typically not a problem for this kind of data.

I wonder if this approach should actually be an official recommendation.

-- 
René Fleschenberg

-- 
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/1856941.AoUJGOvPjS%40rex.
For more options, visit https://groups.google.com/d/optout.

Re: PostGres 9.5 Upsert

2016-01-10 Thread Sean Brant 


> On Jan 10, 2016, at 2:09 AM, Anssi Kääriäinen  wrote:
> 
> If I recall correctly, MySQL doesn't offer a way to specify on which index 
> you want to do the conflict resolution. This leads to problems - the upsert 
> might affect the wrong row if there are multiple unique indexes on the table.
> 
> PostgreSQL's version of upsert has a problem, too. It doesn't offer a direct 
> way to know if the result of the upsert was an insert or update, but Django 
> needs that knowledge, at least for save(). Maybe there is a way (the oid 
> return value seems promising).
> 
> For get_or_create and update_or_create the problem is that the user is free 
> to offer any condition to be used for matching, but PostgreSQL limits the 
> upsert matching to columns in unique index. So, we can use upsert only for 
> unique index cases.

I've always considered this a bug waiting to happen. I have seen many errors 
with the get operation failing because it returned more then one value. Usually 
you don't notice the error until you hit production. I always suggest the 
lookup use fields that have unique indexes.

Changing that would be backwards incompatible so maybe it's a docs issue.

> 
> The save() operation matches the semantics of upsert exactly - maybe we could 
> use upsert there?
> 
>  - Anssi
> 
>> On Sunday, January 10, 2016, Cristiano Coelho  
>> wrote:
>> I agree! Also, does this already happen for the MySQL backend? MySQL has the 
>> insert on conflict update, that could work the same way.
>> However, if I'm not wrong, the docs states that the above methods have a 
>> race condition (obvious since right now it does two operations), but if the 
>> code would actually use native database operations, the race conditions 
>> might be gone for those cases, so that should probably be documented as well.
>> 
>> El viernes, 8 de enero de 2016, 21:13:26 (UTC-3), bliy...@rentlytics.com 
>> escribió:
>>> 
>>> Hey Guys,
>>> 
>>> Postgres 9.5 has added the functionality for UPSERT aka update or insert.  
>>> Any interest in aligning UPSERT on the db layer with the get_or_create or 
>>> update_or_create functionality in django?  Sounds like my company would be 
>>> interested in doing the work if the PR will get the traction.
>>> 
>>> -Ben
>> 
>> -- 
>> 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/ae38ba8e-3e79-47fb-92b9-dd305176c58e%40googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.
> 
> -- 
> 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/CALMtK1Gub-uL3AdgFSxQkdUNNe_Q47%3D7O%3DZ4qsW3Exwjhkd4ZA%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
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/A53126E5-B9D8-4A5A-A56E-B58B77500E94%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: Generic M2M

2016-01-10 Thread Götz Bürkle 
Hi James,

> Generic Foreign Keys is pretty useful, do you think Django could provide a 
> model with 2 GFKs and Generic Many-to-Many fields ?
>
> There is a little old implementation in an app that has been out there for a 
> while so it's pretty old: https://github.com/coleifer/django-generic-m2m/

I just looked into this a few days ago for an application I am
currently working on. Via
http://stackoverflow.com/questions/19772800/django-many-to-many-generic-relationship
I came across the project you mentioned and also another third-party
application that seems to achieve something similar with a slightly
different approach when it comes to table management:
https://bitbucket.org/tkhyn/django-gm2m
(https://pypi.python.org/pypi/django-gm2m)

The Stackoverflow thread references
http://python.6.x6.nabble.com/Generic-ManyToMany-Functionality-td476651.html
- so in 2007, there was support for this kind of feature, just not the
time to implement it.

I also think it would be useful to have it in Django. However, as
there are two third-party applications available that cover this
requirement, I am not sure if it really needs to be integrated into
Django. Maybe other people on the mailing list have a better overview
about how relevant such a feature is.

Kind regards,
Goetz

-- 
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/CAHeTuuVj4mw8pgSdJfR%2Bq3CeaqouU6iA-e4koCBKyK2NDUtG9A%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Generic M2M

2016-01-10 Thread James Pic 
Hi all,

Generic Foreign Keys is pretty useful, do you think Django could provide a
model with 2 GFKs and Generic Many-to-Many fields ?

There is a little old implementation in an app that has been out there for
a while so it's pretty old: https://github.com/coleifer/django-generic-m2m/

We could perhaps have multiple value support for raw_id_fields in the admin
to support it.

Would it be worth working on a DEP for this ?

-- 
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/CALC3KaeVm0OMo3bZH25sR18Jq3M-4RriqMiE%3Dz89usAS5po_wQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: PostGres 9.5 Upsert

2016-01-10 Thread Florian Apolloner 


On Sunday, January 10, 2016 at 9:09:37 AM UTC+1, Anssi Kääriäinen wrote:
>
> The save() operation matches the semantics of upsert exactly - maybe we 
> could use upsert there?
>

Not sure *.save()* would be a good candidate for that. In the best case, 
*.save()* should execute one INSERT or UPDATE query without extra junk for 
"error" handling. Ie if you set a pk and want to update then pass 
*force_update* into it. I know that we already try UPDATE followed by 
INSERT in *save() *but I am wondering how often that is actually used by 
people.

Cheers,
Florian

-- 
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/7edcc120-14b3-4c1b-9afa-8e06fe2156ee%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: PostGres 9.5 Upsert

2016-01-10 Thread Anssi Kääriäinen 
If I recall correctly, MySQL doesn't offer a way to specify on which index
you want to do the conflict resolution. This leads to problems - the upsert
might affect the wrong row if there are multiple unique indexes on the
table.

PostgreSQL's version of upsert has a problem, too. It doesn't offer a
direct way to know if the result of the upsert was an insert or update, but
Django needs that knowledge, at least for save(). Maybe there is a way (the
oid return value seems promising).

For get_or_create and update_or_create the problem is that the user is free
to offer any condition to be used for matching, but PostgreSQL limits the
upsert matching to columns in unique index. So, we can use upsert only for
unique index cases.

The save() operation matches the semantics of upsert exactly - maybe we
could use upsert there?

 - Anssi

On Sunday, January 10, 2016, Cristiano Coelho 
wrote:

> I agree! Also, does this already happen for the MySQL backend? MySQL has
> the insert on conflict update, that could work the same way.
> However, if I'm not wrong, the docs states that the above methods have a
> race condition (obvious since right now it does two operations), but if the
> code would actually use native database operations, the race conditions
> might be gone for those cases, so that should probably be documented as
> well.
>
> El viernes, 8 de enero de 2016, 21:13:26 (UTC-3), bliy...@rentlytics.com
>  escribió:
>>
>> Hey Guys,
>>
>> Postgres 9.5 has added the functionality for UPSERT aka update or
>> insert.  Any interest in aligning UPSERT on the db layer with the
>> get_or_create or update_or_create functionality in django?  Sounds like my
>> company would be interested in doing the work if the PR will get the
>> traction.
>>
>> -Ben
>>
> --
> 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/ae38ba8e-3e79-47fb-92b9-dd305176c58e%40googlegroups.com
> 
> .
> For more options, visit https://groups.google.com/d/optout.
>

-- 
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/CALMtK1Gub-uL3AdgFSxQkdUNNe_Q47%3D7O%3DZ4qsW3Exwjhkd4ZA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.