#12308: Adding tablespace support to postgres backends
-------------------------------------+-------------------------------------
               Reporter:  tclineks   |          Owner:  aaugustin
                   Type:  New        |         Status:  new
  feature                            |      Component:  Database layer
              Milestone:             |  (models, ORM)
                Version:  SVN        |       Severity:  Normal
             Resolution:             |       Keywords:
           Triage Stage:  Accepted   |      Has patch:  1
    Needs documentation:  0          |    Needs tests:  0
Patch needs improvement:  1          |  Easy pickings:  0
                  UI/UX:  0          |
-------------------------------------+-------------------------------------
Changes (by akaariai):

 * needs_better_patch:  0 => 1


Comment:

 Some comments. I can't break it => good!

 There are some places which do something like this:
 {{{
 if tablespace:
     tablespace_sql = self.connection.ops.tablespace_sql(tablespace)
     if tablespace_sql:
         tablespace_sql = ' ' + tablespace_sql
 else:
     tablespace_sql = ''
 }}}

 If I am not mistaken, this could be written as:

 {{{
 if tablespace and connection_supports_tablespaces: # sorry for the pseudo-
 code
     tablespace_sql = ' ' + self.connection.ops.tablespace_sql(tablespace)
 else:
     tablespace_sql = ''
 }}}

 IMHO that makes it a little easier to see what is happening. The default
 implementation of tablespace_sql() could then raise NotImplementedError
 now that there is a flag which tells if the backend supports SQL.
 Currently the flag isn't used at all except in tests. There could be an
 external backend that currently implements tablespace_sql() but will have
 supports_tablespaces flag False when used in Django 1.4 installation. That
 backend would use tablespaces, but not support them.

 If I am not totally mistaken, the db_tablespace kwarg for
 models.ManyToManyField does not cause the intermediary table to be created
 in that db_tablespace. The db_tablespace of the model containing the
 ManyToManyField is used. Wouldn't it be better that one could use the
 db_tablespace in ManyToManyField to define the tablespace of the
 intermediary model? However, that seems to be existing behavior and could
 be solved in another ticket.

 Otherwise looks good. IMHO patch needs improvement based on the added
 supports_tablespaces flag which then is not used otherwise than in tests.

-- 
Ticket URL: <https://code.djangoproject.com/ticket/12308#comment:17>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

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

Reply via email to