Reviewing merge request #138: Table prefix support

I implemented table prefix support for my own use.

The Trac ticket matching this feature is:
http://status.net/trac/ticket/581

I tried to not rewrite too much code.

The first commit implement a generic support directly integrated with DB_DataObject.

Others commit make sure every raw sql query is passed to an utility function before being executed.

I added a SQL script to quickly add prefixes to your current instance.

All lib/classes/actions scripts are covered. scripts and plugins are not, i was not needing it. I'm volunteer do it later if you accept this merge request.

Commits that would be merged:

Version 1
  • Version 1
  • a3314f4
  • 1932b2f

    • table prefix base implementation

  • fe83dd7

    • use table prefix utility function everywhere raw sql is used

  • 6c39383

    • add sql sample script to add table prefix to an existing install

  • 8f29b72

    • fix few more sql queries

  • 26738ae

    • ensure that tablename is prefixed in ensureTable

Showing a3314f4-1932b2f

Comments

Hmm… common_sql_prefix_query looks extremely fragile to me; it could easily miss many perfectly valid table references, as well as damaging any raw strings that might be in the query if they resemble a table name.

I would strongly recommend against this approach; it'd be much more reliable to ensure that the table names actually get run through the existing common_database_tablename() function before being put into queries.

avatar Brion Vibber | 2010-04-07 17:21:34 UTC

It’s ugly yes :–) But i see it as a transitional utility to avoid rewriting all SQL queries. It can’t have any impact on non-prefixed install, so it’s not harmful.

When someone will be volunteer to rewrite this queries, he could search the code base for ‘common_sql_prefix_query’ and fix every query above. Once done, the calls could be safely removed.

For now, it works for my needs and i'm sorry to not have more time to involve on this feature.

avatar François Hodierne | 2010-04-08 22:41:48 UTC

Well, i've thinking about this issue today.

If you (the status.net team) agree to integrate this ‘table-prefix’ feature in your next release and commit to support it on the long term, I'm volunteer to rewrite all raw SQL queries and bypass the transitional utility function.

I just don’t want to waste my time if you have no interest in this feature.

Just let me know!

avatar François Hodierne | 2010-04-09 19:54:31 UTC

Add a new comment:

Login or create an account to post a comment

How to apply this merge request to your repository