RawQuerySet as subquery when used with an __in filter

2012-08-30 Thread Alex Hill
Hi, and thanks for making Django.

At the moment, when passing a RawQuerySet to an __in filter, the raw query 
is evaluated and the result passed as a parameter to the filter's SQL. It 
would be nice to have the option of using the raw SQL in a subquery 
instead, for performance reasons.

I think in this case the rationale for accepting 
https://code.djangoproject.com/ticket/14733 applies. Raw queries already 
require the developer to know what they're doing - this just makes them a 
little more useful.

I can see two ways of implementing this. The first is simple: just require 
that the query only select one column, and implement _as_sql on RawQuerySet 
to simply return the SQL and params. A flag passed to raw() could let a 
developer explicitly indicate that the query should be used as a subquery 
instead of evaluated.

The second way is more complicated but nicer: wrap the raw SQL in another 
SELECT, selecting the Model's primary key. The advantage of this method is 
that the raw query can select many columns, and therefore remain useful in 
other contexts, while still working as expected with __in. This could then 
be the default behaviour, with the current behaviour achieved by wrapping 
the query in list() as is the case for regular queries.

I much prefer the second approach, but I don't think I'm familiar enough 
with the ORM to implement it without a little guidance from someone more 
knowledgeable.

I'd love to hear some feedback on this idea!

Cheers,
Alex

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/django-developers/-/zfVpJLrs7XsJ.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: RawQuerySet as subquery when used with an __in filter

2012-09-06 Thread Alex Hill
Hi Anssi,

Thanks for your feedback! Responses inline.

  1. It should not be assumed that the primary key is the field needed 
> in the inner query. We do have foreign keys to other fields, and one 
> can use this approach without foreign keys at all.


Hmm, yes. Thinking about it, this seems like the biggest obstacle to 
implementing the query-wrapping approach.

The reason I proposed just selecting the PK was to mirror the behaviour of 
QuerySet when passed to __in, as happens here:

https://github.com/django/django/blob/master/django/db/models/query.py#L947 

But since we're talking about raw SQL, we'd want maximum flexibility - so 
would we need to introduce a ValuesRawQuerySet in order to select arbitrary 
columns? It seems that would be the approach that most closely mirrors 
QuerySet.

  2. If the query is of the form NOT IN, then we need to also filter 
> out null values, otherwise no results are returned if the raw query 
> contains a single null (NOT IN is weird...)
>

Yep.
 

>   3. Does the wrapping of the subquery into (select field from 
> (raw_query) where field is not null) cause performance problems? It 
> could be possible that this prevents the DB's optimizer from working 
> correctly.
>

This would need more testing, but Postgres at least optimises the inner 
query away nicely. These two queries have the same execution plan:

SELECT ... WHERE foo NOT IN (SELECT foo FROM (SELECT * FROM table WHERE 
...) WHERE foo IS NOT NULL)
SELECT ... WHERE foo NOT IN (SELECT foo FROM table WHERE ... AND foo IS NOT 
NULL)
 
Same goes for the equivalent IN queries.

  4. If the query doesn't select the required column (that is, it is 
> deferred), we don't have any way to know that. This seems like a 
> documentation issue, but worth mentioning.
>

Yes, definitely. The docs page where .raw() is introduced will have to 
explain this feature with a big warning.
 

> It would be useful to have this feature, so a +1 to this idea. The 
> above issues do not seem too severe, so I believe it is possible to 
> make this work.
>

Me too!

Thanks,
Alex

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/django-developers/-/S11TF_KH5XkJ.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Should annotations allow model field constraints?

2014-12-05 Thread Alex Hill
First, thanks for all your hard work in this area Josh.

>From a quick look at the code, my understanding is that output_field can be 
None, in which case the ORM will use the model's field type, or it can be 
specified explicitly in the query.

I think that if specified explicitly, any constraints should be honoured 
and an exception raised if the computed aggregate value violates a 
constraint. If it's not specified explicitly, any constraints on the model 
field should be ignored as per your reasoning above. I should be able to 
write Model.objects.aggregate(sum_price=Sum('price')), where the sum value 
violates the constraints, without any error.

That provides sensible default behaviour which can be easily overridden if 
necessary.

Cheers,
Alex

On Friday, December 5, 2014 10:04:21 AM UTC+8, Josh Smeaton wrote:
>
> Trac user jbg has found a regression with the aggregation/annotation 
> refactor #14030. The crux of the problem comes down to whether annotations 
> and aggregations should respect constraints on model fields. The specific 
> example raised is as follows:
>
> Model.objects.aggregate(
> sum_price=Sum('price', output_field=DecimalField(max_digits=5, 
> decimal_places=2)))
>
> Assuming the field "price" is defined as "DecimalField(max_digits=5,
>  decimal_places=2)", it doesn't matter if output_field is defined or not, 
> as the implicit F('price') will result in exactly the same behaviour.
>
> Each row in the table will adhere to the max_digits and decimal_places 
> restriction. It has to, it's part of the column definition. But once you 
> sum those values together, they are going to overflow. The same problem 
> would exist if you were to concatenate two CharFields with a max_length=10.
>
> Ticket https://code.djangoproject.com/ticket/23941 has been opened to 
> track this.
>
> These constraints do not necessarily make sense because we're not 
> persisting these values anywhere. They are read-only computations, and 
> should not require bounds. If these values were to be persisted in another 
> model, then the input fields on that model are required to ensure the 
> values are valid.
>
> I've implemented https://github.com/django/django/pull/3655/ 
> 
>  
> which effectively ignores the max_digits and decimal_places arguments in 
> the base expression class. Subclasses are free to enforce validation if it 
> is necessary. Further, callers using aren't required to supply bounds 
> checking in this situation. This is valid (with my patch):
>
> Model.objects.aggregate(
> sum_price=Sum('price', output_field=DecimalField()))
>
> And the same goes with CharField() not requiring a max_length argument.
>
> Can anyone see any issues with this approach?
>
> If the above seems sensible, I'll alter the test cases to remove arguments 
> like decimal_places and max_length, and update the documentation to call 
> out that these kinds of arguments are not used for annotations.
>
> Thanks,
>
> Josh
>
>

-- 
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 http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/3c59d8f2-e771-477c-abd6-9079b182d318%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Should annotations allow model field constraints?

2014-12-05 Thread Alex Hill
Hi Josh,
 

> This is similar to what Ansii thinks the behaviour should be. But we don't 
> know if a user has provided the field, or if the field is being repurposed 
> from the underlying field (for F() expressions), or if it was derived 
> internally.
>

I was thinking you'd instantiate the field without the constraints in 
_resolve_output_field, i.e. when output_field is None. Essentially it's the 
same as the "just provide a class" approach: it would require a contract 
which says all fields must be able to be instantiated in an 
output_field-compatible (i.e. "unconstrained") way by providing no 
arguments.

That seems related to the limitation in DecimalField that we're talking 
about correcting anyway. Do you have a feeling of how near or far Django's 
built-in fields are of meeting that requirement currently?
 

> We could instantiate with no args, but I'm not sure that's a better API.
>

It's nicer at least in the sense that you don't ignore constraints. The 
lack of constraints on the output field is implied by the API. I quite like 
that.

Cheers,
Alex

-- 
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 http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/6f1d1945-104a-49ab-87e4-a3505c09c27d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Abstract models and the app registry

2015-02-11 Thread Alex Hill
Hi Carl,

Thanks for getting back to me. That all makes sense.

I'm still curious as to why abstracts aren't registered – is that a 
requirement of some feature of the app registry design, or is it just that 
way because?

I think what I would like to do is just declare that string references in 
related fields are only resolved in concrete models. In abstract models, 
they can just remain as string references until the fields are copied and 
contributed to concrete subclasses, at which point they'll be resolved 
normally. Then if abstract models do end up in lazy operations, they can be 
considered an anomaly just as a typo would be; we don't need to account for 
them specifically.

Can you see any problem with that approach?

I've just implemented that change and the test suite passes – not 
surprising given that there were edge cases where this was happening anyway 
(e.g. abstract models with relations to "self").

Cheers,
Alex

On Thursday, February 12, 2015 at 2:49:34 AM UTC+8, Carl Meyer wrote:
>
> Hi Alex, 
>
> On 02/11/2015 11:39 AM, Alexander Hill wrote: 
> > I'm looking for some background about abstract models, specifically why 
> > they aren't registered with the app registry as normal models are. 
> > 
> > For some context, I'm working on a ticket [0] to refactor Django's lazy 
> > model operations (used e.g. to resolve string references in related 
> > fields). To acquire an actual model class from an "app_label.ModelName" 
> > string, we use Apps.get_registered_model. However, as far as that method 
> > is concerned, abstract models don't exist, so operations that wait on 
> > them are never run. 
> > 
> > It also means abstract models clutter the pending operations dict, 
> > making it impossible to provide a good warning when there's a typo in a 
> > string model reference, for example. 
> > 
> > Fortunately this doesn't matter too much in practise, because concrete 
> > subclasses work fine despite related fields not being hooked up right on 
> > an abstract parent. 
> > 
> > [0] https://code.djangoproject.com/ticket/24215 
>
> Django treats abstract models as simply a convenient way to group some 
> fields and Python-level behavior; they aren't real models in any sense 
> (they have no db table, you can't query on them, you can't get them from 
> the app registry, etc.) It sounds like the behavior you're describing is 
> mostly consistent with that, except for the "clutter the pending 
> operations dict" part. I would think that ideally abstract models should 
> never appear in the pending operations dict at all in the first place, 
> and any warning regarding a typo in a string model reference should be 
> fired based on a concrete subclass, not based on the abstract base 
> itself. Is there a problem with that approach? (It's been a long time 
> since I looked at that code in detail.) 
>
> Carl 
>
>

-- 
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 http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/5904c936-b06e-4a72-a181-e243f1891e7f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Requesting eyeballs on a change to SQLite schema editor

2015-02-11 Thread Alex Hill
Hi all, doing a bit of yak shaving here.

I'm currently refactoring Django's lazy operations [0], and ran into a bit 
of a snag which led me to make some changes to SQLite's schema editor. All 
the tests pass, but I thought I should bring this up here to get a few more 
eyeballs on my changes since it's a pretty critical bit of Django 
infrastructure.

Here's the relevant 
change: 
https://github.com/AlexHill/django/commit/d27869e3dcb03eccf7c3aa09d911b5206bda9d0a

*Explanation:*
SQLite requires creating a new table for many schema alterations.

At present the process is:

   1. define a dummy model with the updated definition and table name 
   "app_model__new"
   2. create that model's table
   3. copy the data from "app_model" into "app_model__new"
   4. delete the "app_model" table
   5. rename "app_model__new" to "app_model"

Consider a model with a foreign key to self called "parent". In master, 
that field on the dummy model actually points to the original model, with 
the correct table name. After the refactor, the dummy model is internally 
consistent, i.e. its parent field points at itself, with its temporary 
table name. After you create the new table "app_model__new" and rename it 
to "app_model", its definition is left with a dangling reference to 
"app_model__new".

An equivalent way to do this is:

   1. define a dummy model with the updated definition and the existing 
   table name "app_model"
   2. rename "app_model" to "app_model__old"
   3. create the dummy model's table
   4. copy the data from "app_model__old" to "app_model"
   5. delete the "app_model__old" table
   
This avoids ever having a model with a db_table set to the temporary table 
name. As well as fixing my problem, this seems a little more 
straightforward – there's a bit of awkward search-and-replace happening 
with the "__new" table name in the schema editor which we avoid this way.

Cheers,
Alex

[0] https://code.djangoproject.com/ticket/24215

-- 
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 http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/fe83b0cf-d73d-4916-a719-54259b3a1220%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Requesting eyeballs on a change to SQLite schema editor

2015-02-12 Thread Alex Hill
Standalone PR now at https://github.com/django/django/pull/4122.

Alex

On Thursday, February 12, 2015 at 12:09:39 PM UTC+8, Alex Hill wrote:
>
> Hi all, doing a bit of yak shaving here.
>
> I'm currently refactoring Django's lazy operations [0], and ran into a bit 
> of a snag which led me to make some changes to SQLite's schema editor. All 
> the tests pass, but I thought I should bring this up here to get a few more 
> eyeballs on my changes since it's a pretty critical bit of Django 
> infrastructure.
>
> Here's the relevant change: 
> https://github.com/AlexHill/django/commit/d27869e3dcb03eccf7c3aa09d911b5206bda9d0a
>
> *Explanation:*
> SQLite requires creating a new table for many schema alterations.
>
> At present the process is:
>
>1. define a dummy model with the updated definition and table name 
>"app_model__new"
>2. create that model's table
>3. copy the data from "app_model" into "app_model__new"
>4. delete the "app_model" table
>5. rename "app_model__new" to "app_model"
>
> Consider a model with a foreign key to self called "parent". In master, 
> that field on the dummy model actually points to the original model, with 
> the correct table name. After the refactor, the dummy model is internally 
> consistent, i.e. its parent field points at itself, with its temporary 
> table name. After you create the new table "app_model__new" and rename it 
> to "app_model", its definition is left with a dangling reference to 
> "app_model__new".
>
> An equivalent way to do this is:
>
>1. define a dummy model with the updated definition and the existing 
>table name "app_model"
>2. rename "app_model" to "app_model__old"
>3. create the dummy model's table
>4. copy the data from "app_model__old" to "app_model"
>5. delete the "app_model__old" table
>
> This avoids ever having a model with a db_table set to the temporary table 
> name. As well as fixing my problem, this seems a little more 
> straightforward – there's a bit of awkward search-and-replace happening 
> with the "__new" table name in the schema editor which we avoid this way.
>
> Cheers,
> Alex
>
> [0] https://code.djangoproject.com/ticket/24215
>

-- 
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 http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/9d2b55ba-9119-46ca-9718-505d3c437658%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Adding an option on db_index to not create text/varchar_pattern_ops

2015-03-19 Thread Alex Hill
I agree that this is a problem, and I'd like to see more control over 
indexes generally in Django.

However, at first glance I suspect that fixing only your problem would mean 
adding special-case gunk to the Field class in addition to the Postgres 
schema editor where it currently lives[1]. It feels wrong to expand this 
special-casing when other types could benefit from having customisable 
indexes. For instance, the db_index parameter is useless for column types 
that don't work with a btree index, like jsonb and tsvector.

To fix your immediate problem, can't you avoid creating the extra indexes 
by just setting db_index=False, and then use django-json-dbindex to define 
the indexes you DO want?

I would like to see an index API that allowed Field subclasses to specify, 
per-backend, the indexes that should be created if db_index is True. Those 
defaults should be overridable by the user, perhaps by passing a sequence 
of some Index type as the db_index parameter instead of simply True or 
False.

Not all indexes relate to a single field, so it would make sense to also be 
able to register indexes in a model's Meta as well. The Index type should 
be able to use Expressions to create functional and multi-column indexes.

So, that's my wishlist. All that said if you can come up with a patch that 
fixes your problem in a sane way, great! :)

Cheers,
Alex

[1] 
https://github.com/django/django/blob/35b3158d52a5fe51d3b52aec1109a30a73c5abe9/django/db/backends/postgresql_psycopg2/schema.py#L22

On Friday, March 20, 2015 at 3:44:58 AM UTC+8, rodolphe.q...@novapost.fr 
wrote:
>
> As proposed by timgraham in https://code.djangoproject.com/ticket/24507 I 
> open the discussion here.
>
> When adding db_index=True on a CharField Django automatically create 2 
> indexes on some backends as PostgreSQL. But in usage the second index is 
> not always necessary if you never use contains() or similar queries. As you 
> can see here I extracted indexes usages statistics from one of our 
> production server.
>
> The index *foo_bar_email_from_create_like* is never use even if 
> *foo_bar_email_from_create* is, and if we look on our queries this is 
> totally logic and regular. And it's the same for *foo_bar_tech_id* and 
> *foo_bar_user_type*, and it's the same on the other table.
>
> indexrelname  |  idx_scan  | idx_tup_read | 
> idx_tup_fetch
>
> --++--+--
> foo_bar_address_like  |  0 |0 |   
>   0
> foo_bar_current_profile_id|   1846 |  617 |   
> 236
> foo_bar_date_delete   |  0 |0 |   
>   0
> foo_bar_email_from_create |  31209 |90886 |   
>   21903
> foo_bar_email_from_create_like|  0 |0 |   
>   0 
> foo_bar_entity_id |   8026 |28957 |   
>  14
> foo_bar_pkey  | 1258565593 |   1418841848 |
> 1194873240
> foo_bar_site_id   |4495829 |  51000840065 |   
>3564
> foo_bar_tech_id   |   25045160 | 28233693 |  
> 25087324
> foo_bar_tech_id_like  |  0 |0 |   
>   0
> foo_bar_user_type |  21428 |263769329 | 
> 216686751
> foo_bar_user_type_like|  0 |0 |   
>   0
> foo_bar_uuid_like |  0 |0 |   
>   0
> foo_bar_uuid_uniq |   13134415 | 13157636 |  
> 12928178 
>
> A last point is each index on this table is consumming more the 2Gb on 
> disk.
>
> Even if we can suppress the indexes, and this is what we do with (
> https://github.com/novafloss/django-json-dbindex) on bigger one realy 
> problematic, we'd prefer to not create them.
>
> Thanks for your time and this wonderful project Django is.
>

-- 
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 http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/ad3c-dc43-498b-a9f3-382330bbedc2%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Refactor year, month, day lookups?

2015-03-25 Thread Alex Hill
Hi Josh,

With a suitably-defined Month function your example could be:

sales_per_month = Model.objects.annotate(month=Month(F('mydate'))).values(
'month').Aggregate(sales=Sum('sale'))

ISTM there's a bit of overlap between Func and Transform that could be 
cleared up - I'll start a new thread about it.

Cheers,
Alex

On Wednesday, March 25, 2015 at 12:24:44 PM UTC+8, Josh Smeaton wrote:
>
> 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 http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/b120711a-575e-4ecf-a1a3-ca79e2e1d0c4%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Overlap between Func and Transform

2015-03-25 Thread Alex Hill
Hi list,

The thread about reimplementing the date-based lookups[1] reminded me of 
something that was bugging me a couple of days ago.

There's some overlap between Funcs (of arity 1) and Transforms. They seem 
to fundamentally do the same thing – wrap an expression in some arbitrary 
SQL – but with a different interface.

It seems wrong that some functionality can be tied up in a Transform and 
not available as a Func, and vice-versa. For instance, the built-in Length 
function isn't available as a transform, and the built-in contrib.postgres 
Unaccent transform isn't available as a function.

It would be good to be able to easily register functions of arity 1 as 
transforms, using a decorator for example, to make them available in both 
contexts.

Can all possible transforms be defined like this? That is, are transforms 
strictly a special case of functions or can they do more?

Cheers,
Alex

[1] https://groups.google.com/forum/#!topic/django-developers/WYWrQkBJ2hs


-- 
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 http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/79cf613b-d576-4e51-92fd-8c0b5ed60e93%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Unneeded index created with unique=True and db_index=False in TextField

2015-04-14 Thread Alex Hill
I agree this is a bug, and I think it's independent of the discussion of 
customisable indexes.

Postgres creates the necessary index for unique constraints automatically 
as Tommy said. I'm guessing other backends do too, because the explicit 
index creation DDL is omitted when unique is True in the superclass's 
_model_indexes_sql. [1] So too should the xxx_pattern_ops index DDL be 
omitted unless db_index is True.

Line 24 of the linked file currently reads: [2]

if db_type is not None and (field.db_index or field.unique):
# Create the special text index

The xxx_pattern_ops operator classes only help with LIKE and regex queries; 
the unique constraint doesn't need them. I think we can change that to the 
following with no ill effects:

if db_type is not None and field.db_index:

Cheers,
Alex

[1] 
https://github.com/django/django/blob/28e89783254ac0899a26eee324555a9033ccbe9a/django/db/backends/base/schema.py#L852
[2] 
https://github.com/django/django/blob/28e89783254ac0899a26eee324555a9033ccbe9a/django/db/backends/postgresql_psycopg2/schema.py#L24

On Wednesday, April 15, 2015 at 10:37:42 AM UTC+8, Curtis Maloney wrote:
>
> Was the OP referring to the unique index, or the index created for the 
> LIKE lookups?
>
> I was involved in a discussion recently [was there something on list too?] 
> wanting to be able to opt-out of the second index because they knew they 
> didn't need it, and it was _huge_ on their database.
>
> --
> C
>
>
> On 15 April 2015 at 11:58, Tommy Beadle > 
> wrote:
>
>> I believe that Postgres will *always* create an index on a column with a 
>> UNIQUE constraint.
>>
>> regression=> create table yo (id serial primary key, blah varchar(32) 
>> unique);
>> CREATE TABLE
>> regression=> \d yo
>> Table "public.yo"
>>  Column | Type  |
>> Modifiers
>>
>> +---+-
>>  id | integer   | not null default 
>> nextval('yo_id_seq'::regclass)
>>  blah   | character varying(32) | 
>> Indexes:
>> "yo_pkey" PRIMARY KEY, btree (id)
>> "yo_blah_key" UNIQUE CONSTRAINT, btree (blah)
>>
>> regression=> drop index yo_blah_key;
>> ERROR:  cannot drop index yo_blah_key because constraint yo_blah_key on 
>> table yo requires it
>> HINT:  You can drop constraint yo_blah_key on table yo instead.
>>
>>
>> On Tue, Apr 14, 2015 at 9:01 PM, Some Developer > > wrote:
>>
>>> Using Django 1.8, psycopg2 2.6 and PostgreSQL 9.4.1.
>>>
>>> I have a model with a models.TextField(unique=True, db_index=False, 
>>> primary_key=False) field in it.
>>>
>>> I understand that an index is created because of the comment shown in 
>>> this code:
>>>
>>> https://github.com/django/django/blob/master/django/db/
>>> backends/postgresql_psycopg2/schema.py#L17
>>>
>>> but even though the index is suggested for LIKE queries using non C 
>>> locales I would have thought the addition of db_index=False would have 
>>> negated that.
>>>
>>> I feel that this is a bug. An index is not required by PostgreSQL on a 
>>> unique constraint (it may be recommended but that is beside the point) and 
>>> if I explicitly state db_index=False then the Django ORM should remove the 
>>> index even though the index is recommended.
>>>
>>> Thoughts?
>>>
>>> -- 
>>> 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-develop...@googlegroups.com .
>>> To post to this group, send email to django-d...@googlegroups.com 
>>> .
>>> Visit this group at http://groups.google.com/group/django-developers.
>>> To view this discussion on the web visit https://groups.google.com/d/
>>> msgid/django-developers/552DB881.1090006%40googlemail.com.
>>> For more options, visit https://groups.google.com/d/optout.
>>>
>>
>>
>>
>> -- 
>> Grace and Peace,
>> Tommy B.
>>
>> I want to live like I know what I'm leaving.
>> --Switchfoot, "Awakening"
>>  
>> -- 
>> 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-develop...@googlegroups.com .
>> To post to this group, send email to django-d...@googlegroups.com 
>> .
>> Visit this group at http://groups.google.com/group/django-developers.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/django-developers/CAKfxM0LJNbT%2BWtBW0n%3DD9K3QxNjLas7H3t2ZpMWfuaxXh1uxbQ%40mail.gmail.com
>>  
>> 
>> .
>>
>> For more options, visit https://groups.google.com/d/optout.
>>
>
>

-- 
You received this message because yo

Re: Self-referenced template recursion handling

2013-12-08 Thread Alex Hill
Hi all,

If anybody's interested in some background reading, there is an 
implementation of this in Mezzanine, using an "overextends" tag.

https://github.com/stephenmcd/mezzanine/blob/1.4.16/mezzanine/template/loader_tags.py

Cheers,
Alex

On Sunday, December 8, 2013 11:08:12 PM UTC+8, unai wrote:
>
> Hello, 
>
> > given this approach, what if the third party app wants to self-extend 
> > a django admin template for example? 
>
> I'm working on an other solution that instead of relying on loader 
> skipping 
> relies on template skipping. 
>
> Imagine you extend to a self-reference from within a template. All the 
> templates are ignored until the very same template is skipped and it then 
> continues normally. 
>
> This allows for apps to extend other apps and for filesystem templates to 
> extend other TEMPLATE_DIR roots while order is respected. 
>
> Now, the tricky part is to identify a template uniquely. I went for 
> hashing 
> but, as Apollo13 said on IRC, that's just too expensive. After looking for 
> a 
> while, it seems that both filesystem and app loaders work with absolute 
> paths. 
> It seems to me the best way to identify a template uniquely. 
>
> The beauty of the solution is that template skipping is totally relegated 
> to 
> app and filesystem loaders. Of course, some changes in 
> django.template.loader 
> are needed but they are minor. 
>
> I'm currently having some debugging issues and I'm quite busy with other 
> things 
> but I'll try to provide a complete PR this following week. 
>
> If you can think of any problems that this solution would arise don't 
> hesitate 
> in telling ;) 
>
>
> Best wishes, 
>
> Unai Zalakain 
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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 http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/4ad4b787-7182-4cd2-872c-d84343b6ab9b%40googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.


Re: RawQuerySet as subquery when used with an __in filter

2013-12-12 Thread Alex Hill
Hi Anssi,

Getting back to this, a year later...I've created a branch on my fork and 
opened a ticket on Trac.

My feature branch: 
https://github.com/AlexHill/django/compare/master...raw_subqueries

Ticket on Trac, with more responses based on your comments above: 
https://code.djangoproject.com/ticket/21604

Would love to get some feedback.

Cheers,
Alex

On Friday, September 7, 2012 5:34:16 PM UTC+8, Anssi Kääriäinen wrote:
>
> On 6 syys, 11:22, Alex Hill  wrote: 
> > Hi Anssi, 
> > 
> > Thanks for your feedback! Responses inline. 
> > 
> >   1. It should not be assumed that the primary key is the field needed 
> > 
> > > in the inner query. We do have foreign keys to other fields, and one 
> > > can use this approach without foreign keys at all. 
> > 
> > Hmm, yes. Thinking about it, this seems like the biggest obstacle to 
> > implementing the query-wrapping approach. 
> > 
> > The reason I proposed just selecting the PK was to mirror the behaviour 
> of 
> > QuerySet when passed to __in, as happens here: 
> > 
> > https://github.com/django/django/blob/master/django/db/models/query.p... 
>
> > 
> > But since we're talking about raw SQL, we'd want maximum flexibility - 
> so 
> > would we need to introduce a ValuesRawQuerySet in order to select 
> arbitrary 
> > columns? It seems that would be the approach that most closely mirrors 
> > QuerySet. 
>
> The ValuesRawQuerySet might be a good idea. Granted, the use cases for 
> it are hard to come by apart of support for subqueries... One might 
> say we have implemented a really fancy way to do cursor.execute(); 
> cursor.fetchall()... :) 
>
> Another way is to have a pre_subselect hook, and wrap the raw SQL with 
> the right column selected automatically using that. 
>
>  - Anssi 
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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 http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/857f62f4-6009-4b40-a9fe-464371f12290%40googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.


Re: RawQuerySet as subquery when used with an __in filter

2013-12-17 Thread Alex Hill
Hi all,

In implementing this I ran into a bug in the way SQLite returns column 
information: https://code.djangoproject.com/ticket/21603

It affects raw queries in general, but this feature in particular due to 
the way the code generate nested SQL queries.

Cheers,
Alex



On Friday, December 13, 2013 2:35:09 PM UTC+8, Alex Hill wrote:
>
> Hi Anssi,
>
> Getting back to this, a year later...I've created a branch on my fork and 
> opened a ticket on Trac.
>
> My feature branch: 
> https://github.com/AlexHill/django/compare/master...raw_subqueries
>
> Ticket on Trac, with more responses based on your comments above: 
> https://code.djangoproject.com/ticket/21604
>
> Would love to get some feedback.
>
> Cheers,
> Alex
>
> On Friday, September 7, 2012 5:34:16 PM UTC+8, Anssi Kääriäinen wrote:
>>
>> On 6 syys, 11:22, Alex Hill  wrote: 
>> > Hi Anssi, 
>> > 
>> > Thanks for your feedback! Responses inline. 
>> > 
>> >   1. It should not be assumed that the primary key is the field needed 
>> > 
>> > > in the inner query. We do have foreign keys to other fields, and one 
>> > > can use this approach without foreign keys at all. 
>> > 
>> > Hmm, yes. Thinking about it, this seems like the biggest obstacle to 
>> > implementing the query-wrapping approach. 
>> > 
>> > The reason I proposed just selecting the PK was to mirror the behaviour 
>> of 
>> > QuerySet when passed to __in, as happens here: 
>> > 
>> > https://github.com/django/django/blob/master/django/db/models/query.p... 
>>
>> > 
>> > But since we're talking about raw SQL, we'd want maximum flexibility - 
>> so 
>> > would we need to introduce a ValuesRawQuerySet in order to select 
>> arbitrary 
>> > columns? It seems that would be the approach that most closely mirrors 
>> > QuerySet. 
>>
>> The ValuesRawQuerySet might be a good idea. Granted, the use cases for 
>> it are hard to come by apart of support for subqueries... One might 
>> say we have implemented a really fancy way to do cursor.execute(); 
>> cursor.fetchall()... :) 
>>
>> Another way is to have a pre_subselect hook, and wrap the raw SQL with 
>> the right column selected automatically using that. 
>>
>>  - Anssi 
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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 http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/066a36b2-0d7a-4e80-b6b6-93dbf71537b0%40googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.


Re: Displaying (some) M2M fields with through models specified

2014-05-26 Thread Alex Hill
Thanks Russ, I missed #9475 in my search. I'll have a read through those 
tickets...

...and back. I'm leaning towards keeping the API as-is, with 
add/create/remove simply unavailable or raising exceptions if the 
intermediate model doesn't meet the requirements.

A few reasons:
1. As the discussions show, the best form of this API is not obvious, which 
makes me think maybe there isn't a best form of this API.
2. create and add/remove would either have to have different ways of going 
about this or inherent corner cases, since create uses arbitrary keyword 
args, while add/remove use arbitrary positional args. How do you create a 
relationship to a new model which has a field called "extra"?
3. Although an inconsistency would remain in that only some m2m fields can 
use add/create/remove, I think this is a reasonable one. The difference 
between the presence or absence of at least one non-default field is quite 
easy to grasp and explain: either you need to provide extra data, or you 
don't. Having behaviour split along a clear line like that is acceptable 
IMO. The m2m relationships which can and can't be used with the default 
admin would match this.
4. Nothing is possible with this that isn't possible in roughly the same 
amount of code using the existing methods provided by related managers and 
intermediate model managers (albeit arguably less clearly).

All that said I'm not really against changing the API. I just don't see a 
compelling reason to do so, especially absent an API everyone can agree on.

For now, I'm keen to fix what I see as an unreasonable inconsistency: the 
fact that auto-compatible intermediate models can't do everything 
auto-created models can :)


If this works, it's accidental, and I'd be *very* surprised if the final 
> fix is as simple as this. auto_created is involved with the process that 
> determines whether a table is synchronised; so while it probably works if 
> you add `auto_created = True` to a table that has already been 
> synchronised, you'll probably find that it breaks if you do this on an 
> unsynchronised model (with "can't find table" type errors).
>

You're 100% right - it "works" in the sense that once your model is set up 
correctly, you can delete all your custom admin form code and just add 
auto_created = True to your intermediate model's Meta. That's what I just 
did, and it felt great.

This should be possible to auto detect - a manual flag shouldn't be 
> required. There's enough data in the Options object on a model to determine 
> if an m2m instance can be saved without any data beyond the PKs of the two 
> related objects.
>

I thought having the user specify their intention might be a good idea 
because:
- you can check at startup time whether or not things are going to work as 
they expect
- existing projects' behaviour won't change in any way
- these fields would end up being rendered with no way to modify the 
through model's extra fields, which might be a bit baffling. Then again, no 
more baffling than having them just not show up at all like they do now - 
either way it's a trip to Google.

These are all pretty minor so I guess in order to reduce churn they could 
just be ignored. I'm all for fewer pointless options; if you're OK with no 
flag then so am I.


In summary - you're on the right path. There's a few edge cases that need 
> to be addressed, and from a "greedy core developer" point of view, it would 
> be nice to see the *whole* problem solved, not just the subset you've 
> restricted yourself to. However, I'm sure we'll take whatever patch we can 
> get :-)
>

Thanks again for taking a look at this, Russ.

Alex

 
On Monday, May 26, 2014 2:17:10 PM UTC+8, Russell Keith-Magee wrote:
>
> Hi Alex,
>
> Short version - Broadly speaking, this is a feature we've discussed many 
> times over many years, and we've accepted as a good idea in principle. It's 
> logged as ticket #9475 [1]. There is some additional discussion on the 
> original ticket that introduced m2m through models (#6095 [2]). 
>
> The devil is in the details. Your analysis is largely correct, but there 
> are a couple of details in your analysis that need tweaking.
>
> On Mon, May 26, 2014 at 1:55 PM, Alexander Hill 
> 
> > wrote:
>
>> Hi all,
>>
>> Currently, only M2M fields without custom intermediary models can use the 
>> normal Django M2M form machinery. If you define your own intermediary 
>> models, you need to use inlines instead.
>>
>> I would like to allow fields with custom through models to use the 
>> regular M2M field/widget automatically, if the through model meets the 
>> requirement that all its extra fields either have defaults or are nullable. 
>> I'll refer to these as "auto-compatible" models.
>>
>
> It's not *just* about the admin interface, however. Admin only works 
> because there's an underlying API that can be used; we need an API proposal 
> to underpin the m2m widget used by admin. This was the sticking point that 
> prevented this featur

Re: Lazy operations refactor regression with abstract models #25858

2016-02-10 Thread Alex Hill
It looks like we agree that this depending on import order is not on, so we 
have no choice but to break behaviour in some cases.

Option 1 (don't allow relative references) removes support for relative 
references in abstract models for everyone.

Option 2 (resolve references relative to the first concrete inheriting 
class) doesn't remove anybody's existing behaviour, just formalises it and 
clarifies how it's specified. The most anybody has to do to keep doing what 
they're doing is add an app label to a reference.

Option 3 (always resolve references relative to the abstract model) makes 
impossible something that is currently possible: resolving relative 
references in the concrete class's app.

I like option 2. Being able to define a "floating" reference to a 
to-be-implemented model in a different app is a useful feature in abstract 
models.

If we can't agree on that, I'd go with option 1. I don't really see 
abstract models as belonging to an app in the same way concrete models do, 
and that's reflected in Django in various ways. They're not recognised as 
models by the app registry, fields in their concrete subclasses belong to 
the inheriting class (unlike in concrete inheritance), we have app label 
placeholders for related names, etc. I see them more as a blueprint or 
template than a model. Options 1 and 2 both reinforce that distinction, 
option 3 muddies it a bit.

Cheers,
Alex


On Thursday, February 11, 2016 at 5:29:52 AM UTC+8, charettes wrote:
>
> I should have mentioned that this behavior is reproducible since at least
> Django 1.6 and has not been introduced by Django 1.8. I wouldn't be 
> surprised
> if it has always been working before the fix was introduced.
>
> Still, as you mentionned the conditions required to achieve this were 
> really
> convoluted before the lazy operations refactor.
>
> Simon
>
> Le mercredi 10 février 2016 16:11:43 UTC-5, Shai Berger a écrit :
>>
>> On Tuesday 09 February 2016 23:33:50 charettes wrote: 
>> > Hi everyone, 
>> > 
>> > The chosen fix[1] unfortunately introduced a new regression[2]. 
>> > 
>> > It looks like the behavior described in the previous ticket was 
>> possible 
>> > with 
>> > Django 1.8 under certain circumstances[3] where the abstract models 
>> defined 
>> > in a foreign application were derived as concrete models in another 
>> > applications 
>> > before the abstract models relations are resolved (if they are ever 
>> > resolved): 
>> > 
>>
>> The explanation is complex enough, the case where it works edgy enough, 
>> that 
>> I'd be content to call this a bug in 1.8. I'm pretty sure the specifics 
>> of this 
>> resolution are not documented, and my sense from reading the ticket is 
>> that if 
>> you'd try to document the behavior you'll reach the conclusion that it 
>> probably isn't intentional. 
>>
>> My 2 cents, 
>> Shai. 
>>
>> > 
>> > [1] 
>> https://github.com/django/django/commit/bc7d201bdbaeac14a49f51a9ef292d6 
>> > [2] https://code.djangoproject.com/ticket/26186 
>> > [3] https://code.djangoproject.com/ticket/26186#comment:8 
>> > [4] https://code.djangoproject.com/ticket/24215 
>>
>

-- 
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/ac07321f-aa6a-47dc-b0f1-d49ba9b71d1c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Extend support for long surnames in Django Auth

2016-08-10 Thread Alex Hill
FWIW I agree with Florian:

   - Where the default is unsuitable for a project, it's easier to restrict 
   the field's length in forms than to increase it in the User model.
   - It's hard to imagine a situation where a 100-character limit is 
   suitable but a 150-character limit isn't.
   - I can imagine myself as a new user wasting cycles wondering what the 
   reason behind the different field lengths is, so I imagine others might too.

Cheers,
Alex

On Thursday, August 11, 2016 at 3:43:05 AM UTC+8, Florian Apolloner wrote:
>
> On Wednesday, August 10, 2016 at 5:34:44 PM UTC+2, Tom Christie wrote:
>>
>> I'd always defer towards humanized limits, rather than technical limits, 
>> so I'd suggest 100 chars seems like a decent cap.
>>
>
> Not trying to troll or derail, but can we please make it 150 chars? It is 
> still kinda "humanized" and at least consistent with what we already have 
> for the username. Having a different somewhat arbitrary field for every 
> charfield on the usermodel strikes me as somewhat suboptimal.
>
> 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/1ea26098-3ed0-4a86-b8d3-2fc4d7ec87c8%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.