About: Feature request: Comments in Django models saved to database schema

Showing 1-7 of 7 messages
About: Feature request: Comments in Django models saved to database schema Andrei Antoukh 7/7/12 12:25 PM
Hello!

I am writing about the issues:

I am not the author of these issues, but I think this feature is interesting to have in the ORM.
I agree that we should not do things just because we can make!

In the source code often put comments to clarify things. These comments for the same project 
developer or someone who looks at the source code are often useful to know how 
something works or understand something that is not entirely clear.

In SQL's absolutely the same case. Sometimes you need to make comments on both tables to clarify 
the purpose and functionality, and in columns. All assume that if you want to understand something,
 look at the source code, but this may not always be so.

Right now, the behavior to make comments can be emulated using sql fixtures. And it has these drawbacks:

* Is not portable between different database backends, 3 files need to write redundant for this in mysql, 
postgresql and sqlite (sqlite does not natively support comments, but you can emulate sql 
http://stackoverflow.com/questions/7426205/sqlite comments -adding-comments-to-tables-and-columns )

* It is more tedious to write SQL statements and you end up not making the comments.

Is a low priority feature (obviously) but I think it's something that could facilitate the ORM.

My first proposal is:
Comments to model tables (for mysql, postgresql and sqlite3) as the first patch. 
Without any kind of magic by django, a simple attribute "comment" on object "Meta".

My initial proposal for postgresql implementation: https://github.com/niwibe/django/tree/issue_18468

Andrei Antoukh

PD: sorry for my poor inglish

--
Andrei Antoukh - <ni...@niwi.be>
http://www.niwi.be/page/about/

"Linux is for people who hate Windows, BSD is for people who love UNIX"
"Social Engineer -> Because there is no patch for human stupidity"

Re: About: Feature request: Comments in Django models saved to database schema Anssi Kääriäinen 7/8/12 11:56 PM
On 7 heinä, 22:25, Andrei Antoukh <n...@niwi.be> wrote:
> Hello!
>
> I am writing about the issues:https://code.djangoproject.com/ticket/13867https://code.djangoproject.com/ticket/18468
I am not sure this is something we want to support. To me a generic
"get_post_sync_sql(connection)" is a better approach. In addition to
comments you could add indexes, constraints and so on in the hook. It
should be called even for non-managed tables. That way you could use
views using the hook. Trivial example:

class UserPostCount(models.Model):
    user = models.OneToOneKey(User, primary_key=True,
db_column="user_id", on_delete=models.DO_NOTHING,
related_name='post_count')
    post_count = models.IntegerField()

    class Meta:
        db_table = 'user_post_count_view'
        managed = False

    def get_post_sync_sql(connection):
        if connection.vendor != 'postgresql':
            raise NotImplementedError
        return [
    """
    CREATE OR REPLACE VIEW user_post_count_view AS (
        SELECT user.id AS user_id, count(*) AS post_count
          FROM user
          JOIN user_posts ON user.id = user_posts.user_id
    );""",
    """
    COMMENT ON VIEW user_post_count_view IS 'A trivial view returning
post count for users.';
    """]

There could also be a get_post_flush_sql(connection) hook.

It seems best if the hooks are called in a second pass - that is, the
whole schema is already installed into the DB before running any of
the post_sync SQL.

Why not use the initial SQL files we already have? They are ran every
time after flush - which means they are useless for defining views for
example - and at least I like to keep the SQL together with model
definition. In addition (IIRC) there are some problems with backends
which do not support executing multiple statements in one go. Still,
one can do logic in the hook, while the initial SQL files are flat
files.

 - Anssi
Re: About: Feature request: Comments in Django models saved to database schema Andrei Antoukh 7/9/12 7:58 AM
2012/7/9 Anssi Kääriäinen <anssi.ka...@thl.fi>
Hi Anssi!

I had submitted a proposal very simplistic. But your idea seems great! In fact I love it! And I would solve much more. As you've said: indexes, constrains, etc..
I will present a proposal for implementation with this in mind. 

Andrei Antoukh
 
--
Andrei Antoukh - <ni...@niwi.be>
http://www.niwi.be/page/about/
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-d...@googlegroups.com.
To unsubscribe from this group, send email to django-develop...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: About: Feature request: Comments in Django models saved to database schema Andrei Antoukh 7/22/12 1:01 PM
Hello!

I started working on what had been proposed. In the commit https://github.com/niwibe/django/commit/af887029integeintege614ca53d8bdab23f0a40ee7420b9bc20 is the initial implementation. It is based on the Anssi idea.

I have some doubts about how to make things better. The proposed method should return a list of sql statements:
["COMMENT ON TABLE fooapp_sampletable IS 'Foo comment';"]

or a list of tuples with sql statements with possible arguments:

[("COMMENT ON TABLE fooapp_sampletable IS %s", ["Foo comment"])]

Currently, the initial implementation, is a list of tuples, but I'm wondering if the best option. I think that this is not usabe for commands such as sqlall because the returned sql is not complete, if only partial, failing to place them arguments.

Thank you very much!
Andrey Antukh



2012/7/9 Andrei Antoukh <ni...@niwi.be>

"Linux is for people who hate Windows, BSD is for people who love UNIX"
"Social Engineer -> Because there is no patch for human stupidity"

Re: About: Feature request: Comments in Django models saved to database schema Anssi Kääriäinen 7/24/12 4:40 AM
On 22 heinä, 23:01, Andrei Antoukh <n...@niwi.be> wrote:
> Hello!
>
> I started working on what had been proposed. In the commithttps://github.com/niwibe/django/commit/af887029integeintege614ca53d8...<https://github.com/niwibe/django/commit/af887029614ca53d8bdab23f0a40e...>
> is
> the initial implementation. It is based on the Anssi idea.
>
> I have some doubts about how to make things better. The proposed method
> should return a list of sql statements:
> ["COMMENT ON TABLE fooapp_sampletable IS 'Foo comment';"]
>
> or a list of tuples with sql statements with possible arguments:
>
> [("COMMENT ON TABLE fooapp_sampletable IS %s", ["Foo comment"])]
>
> Currently, the initial implementation, is a list of tuples, but I'm
> wondering if the best option. I think that this is not usabe for commands
> such as sqlall because the returned sql is not complete, if only partial,
> failing to place them arguments.

I do think the sqlall output should be usable as is. That is, the
return value should be list of strings. This might mean that one has
to create the SQL in a way that is susceptible to SQL injection
attacks, but I don't see that as a problem. The post_sync method's SQL
can't be ran or controlled by users, so there is no attack vector.

I have a couple of questions:
  1) How does this work in model inheritance situations? If the base
model defines post_sync_sql(), wouldn't that be called again for the
child models? Maybe it is possible to run the method only if it is
defined in the current class directly. This should be possible by
inspecting the model's __dict__ for the method instead of using
hasattr.

  2) What is the order in which the post_sync_sql methods are called?
This could be important for some use cases (one view depends on
another view for example). I guess the order is app for app in
INSTALLED_APPS, models in the order they are found from app's
models.py? The order should be tested and documented.

 - Anssi
Re: About: Feature request: Comments in Django models saved to database schema Andrei Antoukh 7/26/12 8:55 AM
2012/7/24 Anssi Kääriäinen <anssi.ka...@thl.fi>

On 22 heinä, 23:01, Andrei Antoukh <n...@niwi.be> wrote:
> Hello!

Hello!
 
>
> I started working on what had been proposed. In the commithttps://github.com/niwibe/django/commit/af887029integeintege614ca53d8...<https://github.com/niwibe/django/commit/af887029614ca53d8bdab23f0a40e...>
> is
> the initial implementation. It is based on the Anssi idea.
>
> I have some doubts about how to make things better. The proposed method
> should return a list of sql statements:
> ["COMMENT ON TABLE fooapp_sampletable IS 'Foo comment';"]
>
> or a list of tuples with sql statements with possible arguments:
>
> [("COMMENT ON TABLE fooapp_sampletable IS %s", ["Foo comment"])]
>
> Currently, the initial implementation, is a list of tuples, but I'm
> wondering if the best option. I think that this is not usabe for commands
> such as sqlall because the returned sql is not complete, if only partial,
> failing to place them arguments.

I do think the sqlall output should be usable as is. That is, the
return value should be list of strings. This might mean that one has
to create the SQL in a way that is susceptible to SQL injection
attacks, but I don't see that as a problem. The post_sync method's SQL
can't be ran or controlled by users, so there is no attack vector.
 
OK!

 

I have a couple of questions:
  1) How does this work in model inheritance situations? If the base
model defines post_sync_sql(), wouldn't that be called again for the
child models? Maybe it is possible to run the method only if it is
defined in the current class directly. This should be possible by
inspecting the model's __dict__ for the method instead of using
hasattr.

Good observation! 


  2) What is the order in which the post_sync_sql methods are called?
This could be important for some use cases (one view depends on
another view for example). I guess the order is app for app in
INSTALLED_APPS, models in the order they are found from app's
models.py? The order should be tested and documented.

I'll think about it. Currently uses the same order as syncdb, even if not the most appropriate for this.  
We could think of a way to define the order of execution by the user.

I created the asociated ticket for this feature: https://code.djangoproject.com/tidcket/18672

Andrey.

 

 - Anssi

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



Re: About: Feature request: Comments in Django models saved to database schema Anssi Kääriäinen 7/26/12 9:16 AM
On 26 heinä, 18:55, Andrei Antoukh <n...@niwi.be> wrote:
> 2012/7/24 Anssi Kääriäinen <anssi.kaariai...@thl.fi>
> I created the asociated ticket for this feature:https://code.djangoproject.com/tidcket/18672<https://code.djangoproject.com/ticket/18672>
>
> Andrey.

Before proceeding too far in the morel pre/post sync hooks, lets see
if the other current thread about a similar issues yields a better
API. The other thread is about raw SQL on pre/post sync, and while it
isn't dealing with the exact same issue as this thread, there is a lot
of overlap. See http://groups.google.com/group/django-developers/browse_thread/thread/c4dd1a333ae60356

 - Anssi