[Trac] Re: PostgreSQL Conversion

[Brad Anderson]

On Friday 14 January 2005 1:08 am, Justus Pendleton wrote:
> On 2005-01-03, Brad Anderson <brad at dsource.org> wrote:
> > This weekend, I tinkered with a patch to allow Trac to work with a
> > PostgreSQL database instead of SQLite.  It is just the beginning of what
> > is required, but I got most modules to at least come up and render data.
>
> Brad,
>
> I skimmed over your patch and had a few comments:

First off, thanks for taking the time.  There are precious few on this project 
doing that, and I feel remiss in not perusing your darcs patch yet...

>
> * For command line initenv usage I notice you've just added some more
>   positional parameters, which is what I also did for new version control
>   backends.  I think that someone eventually needs to rework initenv so
>   that it uses normal getopt stuff to make these kinds of things easier to
>   handle: e.g. what if someone wants darcs and postgres?  I'm not saying
>   this necessarily belongs in your postgres patch, it was just something I
>   noticed.

Yeah, I thought of that too.  What about making returnvals[3] another type of 
structure (tuple, dict)?  I didn't want to require the user to type in the 
db_str, like

# pgsql:"",host='localhost:5432',database='trac',user='trac',password='trac'

but rather ask more questions.  

Then we could reserve returnvals[1] for SCCM.  Or just do the getopt() 
refactoring like you suggest.

>
> * What happens if the user types something other than pgsql for the
>   database system?  I might be wrong but it looks as if any string that
>   isn't "pgsql" is treated as sqlite.  I guess this is more along the lines
>   of the pluggable modules but it would be cool if database support were a
>   pluggable module.  You could query the installed modules to get a list of
>   valid strings for database system.  The individual modules could even be
>   smart enough not to register themselves if, for instance, you don't have
>   pysqlite installed.

I probably didn't trap this, but was thinking that before I came along with 
Postgres, it was assumed to be sqlite.  I should probably require one of the 
approved values.  

As for making the db a pluggable module, this may be difficult, and it may 
not.  Each pluginhas its own SQL statements.  I'm trying to make them generic 
enough to work across sqlite, pgsql, and mysql.  However, when more dbms's 
are added, we may need to have a lookup table of sql statements, and then I 
can see the db module for pgsql filling out the appropriate SQL statement, 
and the mysql plugin filling out a different syntax for the same stmt.  The 
problem with this, then, is that there is a central place for SQL statements, 
and a bunch of disparate plugins.  What if my "different" ticket plugin 
doesn't use the same queries as the "basic" ticket plugin?

>
> * In your motivations for this you mentioned performance.  trac doesn't
>   use any indexes does it?  If not, we might want to add that for both
>   sqlite and other db backends.

Agreed.  Although there was some question when I last thought about this as to 
uniqueness.  I think there are primary keys on the pgsql patch, but no other 
indexes on, say, text fields that are heavily used.

>
> * In my darcs patch I've changed the revision id in the schema to be text
>   rather than an int.  If we changed it to be text even for SVN would that
>   help clean up any of those casts and hacks you have?  Or does it make
>   things worse?

I don't think so.  There are too many other 'id' fields that should be int for 
performance reasons.  I'd even go as far as saying that the whole data model 
would be better served with surrogate keys, and then you're free to make an 
id column that is text for darcs, or int for svn.  The surrogate key would be 
an int and not have any business meaning at all.  Strictly there for speed at 
the query level.

>
> * It looks like you removed the exception trac raises when the web server
>   doesn't have read/write access to the db directory.  Does sqlite just
>   raise an exception under those circumstances?

I think that's just the way the diff utility works.  It sort of mangled the 
get_dbms() and get_db_cnx() functions according to the differences, and so 
where it shows '-' as removed, there's a section down below with '+'.  At 
least that's the way it is in Eclipse.  It's still there in the sqlite 
section of get_db_cnx().

Thanks again for the comments.  Real job is flaring up big-time, but hopefully 
I'll have time to add some things from this mail and fold MySQL into the mix.

I feel like trac-admin needs a bunch of work as well.  I'd like to share 
common functions with the WebAdmin module I worked up earlier.

BA


>
> _______________________________________________
> Trac mailing list
> Trac at lists.edgewall.com
> http://lists.edgewall.com/mailman/listinfo/trac