Changeset 1224

Timestamp:

11/13/05 19:44:35 (3 years ago)

Author:

adrian

Message:

Fixed #121 -- Django now quotes all names in SQL queries. Also added unit tests to confirm. Thanks, Robin Munn and Sune.

Files:

• django/trunk/django/bin/daily_cleanup.py (modified) (1 diff)
• django/trunk/django/core/db/backends/ado_mssql.py (modified) (1 diff)
• django/trunk/django/core/db/backends/postgresql.py (modified) (1 diff)
• django/trunk/django/core/management.py (modified) (19 diffs)
• django/trunk/django/core/meta/__init__.py (modified) (22 diffs)
• django/trunk/django/models/auth.py (modified) (1 diff)
• django/trunk/docs/model-api.txt (modified) (4 diffs)
• django/trunk/tests/testapp/models/__init__.py (modified) (1 diff)
• django/trunk/tests/testapp/models/reserved_names.py (added)

django/trunk/django/bin/daily_cleanup.py

@@ -8 +8 @@
-    # Clean up old database records
-    cursor = db.cursor()
-
-
+
+
+
+
-    db.commit()
-

django/trunk/django/core/db/backends/ado_mssql.py

@@ -150 +150 @@
-    'OneToOneField':     'int',
-    'PhoneNumberField':  'varchar(20)',
-name)s] CHECK ([%(name
-name)s] CHECK ([%(name
+column)s] CHECK ([%(column
+column)s] CHECK ([%(column
-    'SlugField':         'varchar(50)',
-    'SmallIntegerField': 'smallint',

django/trunk/django/core/db/backends/postgresql.py

@@ -171 +171 @@
-    'OneToOneField':     'integer',
-    'PhoneNumberField':  'varchar(20)',
-name
-name
+column
+column
-    'SlugField':         'varchar(50)',
-    'SmallIntegerField': 'smallint',

django/trunk/django/core/management.py

@@ -20 +20 @@
-
-def _get_packages_insert(app_label):
-
+
+
+
+
-
-def _get_permission_codename(action, opts):
@@ -34 +37 @@
-
-def _get_permission_insert(name, codename, opts):
-
-
+
+
+
+
-
-def _get_contenttype_insert(opts):
-
-
+
+
+
+
-
-def _is_valid_dir_name(s):
@@ -65 +72 @@
-            col_type = db.DATA_TYPES[data_type]
-            if col_type is not None:
-f.column
+db.db.quote_name(f.column)
-                field_output.append('%sNULL' % (not f.null and 'NOT ' or ''))
-                if f.unique:
@@ -73 +80 @@
-                if f.rel:
-                    field_output.append('REFERENCES %s (%s)' % \
-
+
+
-                table_output.append(' '.join(field_output))
-        if opts.order_with_respect_to:
-_order %s NULL' % db.DATA_TYPES['IntegerField']
+%s %s NULL' % (db.db.quote_name('_order'), db.DATA_TYPES['IntegerField'])
-        for field_constraints in opts.unique_together:
-
-
-
+
+
+
+
-        for i, line in enumerate(table_output): # Combine and add commas.
-            full_statement.append('    %s%s' % (line, i < len(table_output)-1 and ',' or ''))
@@ -89 +98 @@
-        opts = klass._meta
-        for f in opts.many_to_many:
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-            table_output.append(');')
-            final_output.append('\n'.join(table_output))
@@ -115 +132 @@
-        if cursor is not None:
-            # Check whether the table exists.
-django_admin_log LIMIT 1"
+%s LIMIT 1" % db.db.quote_name('django_admin_log')
-    except:
-        # The table doesn't exist, so it doesn't need to be dropped.
@@ -130 +147 @@
-            if cursor is not None:
-                # Check whether the table exists.
-klass._meta.db_table
+db.db.quote_name(klass._meta.db_table)
-        except:
-            # The table doesn't exist, so it doesn't need to be dropped.
-            db.db.rollback()
-        else:
-klass._meta.db_table
+db.db.quote_name(klass._meta.db_table)
-
-    # Output DROP TABLE statements for many-to-many tables.
@@ -143 +160 @@
-            try:
-                if cursor is not None:
-f.get_m2m_db_table(opts
+db.db.quote_name(f.get_m2m_db_table(opts)
-            except:
-                db.db.rollback()
-            else:
-f.get_m2m_db_table(opts
+db.db.quote_name(f.get_m2m_db_table(opts)
-
-    app_label = mod._MODELS[0]._meta.app_label
-
-    # Delete from packages, auth_permissions, content_types.
-
-
-
+
+
+
+
+
+
-
-    # Delete from the admin log.
-    if cursor is not None:
-
+
+
+
-        if admin_log_exists:
-            for row in cursor.fetchall():
-
+
+
-
-    # Close database connection explicitly, in case this output is being piped
@@ -207 +230 @@
-def get_sql_sequence_reset(mod):
-    "Returns a list of the SQL statements to reset PostgreSQL sequences for the given module."
-
+db, 
-    output = []
-    for klass in mod._MODELS:
-        for f in klass._meta.fields:
-            if isinstance(f, meta.AutoField):
-
+
+
+
-    return output
-get_sql_sequence_reset.help_doc = "Prints the SQL statements for resetting PostgreSQL sequences for the given model module name(s)."
@@ -219 +244 @@
-def get_sql_indexes(mod):
-    "Returns a list of the CREATE INDEX SQL statements for the given module."
+    from django.core.db import db
-    output = []
-    for klass in mod._MODELS:
@@ -225 +251 @@
-                unique = f.unique and "UNIQUE " or ""
-                output.append("CREATE %sINDEX %s_%s ON %s (%s);" % \
-
+
+
-    return output
-get_sql_indexes.help_doc = "Prints the CREATE INDEX SQL statements for the given model module name(s)."
@@ -243 +270 @@
-
-    # Check that the package exists in the database.
-
+
+
-    if cursor.rowcount < 1:
-#         sys.stderr.write("The '%s' package isn't installed.\n" % app_label)
@@ -257 +285 @@
-        contenttypes_seen[opts.module_name] = 1
-        for codename, name in perms:
-
+
+
+
-            if cursor.rowcount < 1:
-#                 sys.stderr.write("The '%s.%s' permission doesn't exist.\n" % (app_label, codename))
-                print _get_permission_insert(name, codename, opts)
-
+
+
+
-        if cursor.rowcount < 1:
-#             sys.stderr.write("The '%s.%s' content type doesn't exist.\n" % (app_label, opts.module_name))
@@ -268 +300 @@
-    # Check that there aren't any *extra* permissions in the DB that the model
-    # doesn't know about.
-
+
+
+
-    for row in cursor.fetchall():
-        try:
@@ -274 +308 @@
-        except KeyError:
-#             sys.stderr.write("A permission called '%s.%s' was found in the database but not in the model.\n" % (app_label, row[0]))
-
+
+
+
-
-    # Check that there aren't any *extra* content types in the DB that the
-    # model doesn't know about.
-
+
+
+
-    for row in cursor.fetchall():
-        try:
@@ -284 +322 @@
-        except KeyError:
-#             sys.stderr.write("A content type called '%s.%s' was found in the database but not in the model.\n" % (app_label, row[0]))
-
+
+
+
-database_check.help_doc = "Checks that everything is installed in the database for the given model module name(s) and prints SQL statements if needed."
-database_check.args = APP_ARGS
@@ -319 +359 @@
-        for sql in get_sql_create(core) + get_sql_create(auth) + get_sql_initial_data(core) + get_sql_initial_data(auth):
-            cursor.execute(sql)
-
+
+
+
-    except Exception, e:
-        sys.stderr.write("Error: The database couldn't be initialized.\n%s\n" % e)
@@ -688 +730 @@
-    index_output = []
-    for f in fields:
-f.column
+db.db.quote_name(f.column)
-        field_output.append("%sNULL" % (not f.null and "NOT " or ""))
-        if f.unique:
@@ -696 +738 @@
-        if f.db_index:
-            unique = f.unique and "UNIQUE " or ""
-
+
+
+
-        table_output.append(" ".join(field_output))
-tablename
+db.db.quote_name(tablename)
-    for i, line in enumerate(table_output):
-        full_statement.append('    %s%s' % (line, i < len(table_output)-1 and ',' or ''))

django/trunk/django/core/meta/__init__.py

@@ -58 +58 @@
-
-def orderlist2sql(order_list, opts, prefix=''):
+    if prefix.endswith('.'):
+        prefix = db.db.quote_name(prefix[:-1]) + '.'
-    output = []
-    for f in handle_legacy_orderlist(order_list):
-        if f.startswith('-'):
-orderfield2column(f[1:], opts
+db.db.quote_name(orderfield2column(f[1:], opts)
-        elif f == '?':
-            output.append(db.get_random_function_sql())
-        else:
-orderfield2column(f, opts
+db.db.quote_name(orderfield2column(f, opts)
-    return ', '.join(output)
-
@@ -786 +788 @@
-    if pk_set:
-        # Determine whether a record with the primary key already exists.
-
+
+
-        # If it does already exist, do an UPDATE.
-        if cursor.fetchone():
-            db_values = [f.get_db_prep_save(f.pre_save(getattr(self, f.attname), False)) for f in non_pks]
-
-
+
+
+
+
-                db_values + [pk_val])
-        else:
-            record_exists = False
-    if not pk_set or not record_exists:
-f.column
+db.db.quote_name(f.column)
-        placeholders = ['%s'] * len(field_names)
-        db_values = [f.get_db_prep_save(f.pre_save(getattr(self, f.attname), True)) for f in opts.fields if not isinstance(f, AutoField)]
-        if opts.order_with_respect_to:
-'_order'
+db.db.quote_name('_order')
-            # TODO: This assumes the database supports subqueries.
-            placeholders.append('(SELECT COUNT(*) FROM %s WHERE %s = %%s)' % \
-opts.db_table, opts.order_with_respect_to.column
+db.db.quote_name(opts.db_table), db.db.quote_name(opts.order_with_respect_to.column)
-            db_values.append(getattr(self, opts.order_with_respect_to.attname))
-
-
+
+
+
-        if opts.has_auto_field:
-            setattr(self, opts.pk.attname, db.get_last_insert_id(cursor, opts.db_table, opts.pk.column))
@@ -833 +839 @@
-                sub_obj.delete()
-    for rel_opts, rel_field in opts.get_all_related_many_to_many_objects():
-
-
+
+
+
-    for f in opts.many_to_many:
-
+
+
+
-            [getattr(self, opts.pk.attname)])
-
+
+
+
-    db.db.commit()
-    setattr(self, opts.pk.attname, None)
@@ -855 +866 @@
-    if not hasattr(self, '_next_in_order_cache'):
-        self._next_in_order_cache = opts.get_model_module().get_object(order_by=('_order',),
-
-
+
+
+
+
-            params=[getattr(self, opts.pk.attname), getattr(self, order_field.attname)])
-    return self._next_in_order_cache
@@ -863 +876 @@
-    if not hasattr(self, '_previous_in_order_cache'):
-        self._previous_in_order_cache = opts.get_model_module().get_object(order_by=('-_order',),
-
-
+
+
+
+
-            params=[getattr(self, opts.pk.attname), getattr(self, order_field.attname)])
-    return self._previous_in_order_cache
@@ -889 +904 @@
-    if not hasattr(self, cache_var):
-        mod = rel.get_model_module()
-
-
-
-
+
+
+
+
+
+
+
-        cursor = db.db.cursor()
-        cursor.execute(sql, [getattr(self, self._meta.pk.attname)])
@@ -917 +935 @@
-    this_id = getattr(self, self._meta.pk.attname)
-    if ids_to_delete:
-
+
+
+
+
-        cursor.execute(sql, [this_id])
-    if ids_to_add:
-
+
+
+
+
-        cursor.executemany(sql, [(this_id, i) for i in ids_to_add])
-    db.db.commit()
@@ -966 +990 @@
-    this_id = getattr(self, self._meta.pk.attname)
-    cursor = db.db.cursor()
-
-
+
+
+
+
+
+
+
-    cursor.executemany(sql, [(this_id, i) for i in id_list])
-    db.db.commit()
@@ -976 +1005 @@
-    cursor = db.db.cursor()
-    # Example: "UPDATE poll_choices SET _order = %s WHERE poll_id = %s AND id = %s"
-
+
+
+
+
-    rel_val = getattr(self, ordered_obj.order_with_respect_to.rel.field_name)
-    cursor.executemany(sql, [(i, rel_val, j) for i, j in enumerate(id_list)])
@@ -984 +1016 @@
-    cursor = db.db.cursor()
-    # Example: "SELECT id FROM poll_choices WHERE poll_id = %s ORDER BY _order"
-
+
+
+
+
+
-    rel_val = getattr(self, ordered_obj.order_with_respect_to.rel.field_name)
-    cursor.execute(sql, [rel_val])
@@ -994 +1030 @@
-    op = is_next and '>' or '<'
-    kwargs.setdefault('where', []).append('(%s %s %%s OR (%s = %%s AND %s %s %%s))' % \
-
+
+
-    param = str(getattr(self, field.attname))
-    kwargs.setdefault('params', []).extend([param, param, getattr(self, opts.pk.attname)])
@@ -1081 +1118 @@
-
-def _get_where_clause(lookup_type, table_prefix, field_name, value):
+    if table_prefix.endswith('.'):
+        table_prefix = db.db.quote_name(table_prefix[:-1])+'.'
+    field_name = db.db.quote_name(field_name)
-    try:
-        return '%s%s %s %%s' % (table_prefix, field_name, db.OPERATOR_MAPPING[lookup_type])
@@ -1161 +1201 @@
-    cursor = db.db.cursor()
-    _, sql, params = function_get_sql_clause(opts, **kwargs)
-opts.db_table, f
+db.db.quote_name(opts.db_table), db.db.quote_name(f)
-    cursor.execute("SELECT " + (kwargs.get('distinct') and "DISTINCT " or "") + ",".join(select) + sql, params)
-    while 1:
@@ -1182 +1222 @@
-            db_table = f.rel.to.db_table
-            if db_table not in cache_tables_seen:
-_table
+.db.quote_name(db_table)
-            else: # The table was already seen, so give it a table alias.
-                new_prefix = '%s%s' % (db_table, len(cache_tables_seen))
-_table, new_prefix
+.db.quote_name(db_table), db.db.quote_name(new_prefix)
-                db_table = new_prefix
-            cache_tables_seen.append(db_table)
-
-
+
+
+
+
-            _fill_table_cache(f.rel.to, select, tables, where, db_table, cache_tables_seen)
-
@@ -1248 +1290 @@
-                for f in current_opts.many_to_many:
-                    if f.name == current:
-'t%s' % table_count
+db.db.quote_name('t%s' % table_count)
-                        table_count += 1
-
-
-
+
+
+
+
+
+
+
-                        # Optimization: In the case of primary-key lookups, we
-                        # don't have to do an extra join.
@@ -1263 +1309 @@
-                        else:
-                            new_table_alias = 't%s' % table_count
-
-
-
+
+
+
+
+
+
+
-                            current_table_alias = new_table_alias
-                            param_required = True
@@ -1288 +1338 @@
-                        else:
-                            new_table_alias = 't%s' % table_count
-
-
-
+
+
+
+
+
-                            current_table_alias = new_table_alias
-                            param_required = True
@@ -1309 +1361 @@
-
-def function_get_sql_clause(opts, **kwargs):
-opts.db_table, f.column
+db.db.quote_name(opts.db_table), db.db.quote_name(f.column)
-    tables = [opts.db_table] + (kwargs.get('tables') and kwargs['tables'][:] or [])
+    tables = [db.db.quote_name(t) for t in tables]
-    where = kwargs.get('where') and kwargs['where'][:] or []
-    params = kwargs.get('params') and kwargs['params'][:] or []
@@ -1329 +1382 @@
-    # Add any additional SELECTs passed in via kwargs.
-    if kwargs.get('select'):
-s[1], s[0]
+db.db.quote_name(s[1]), db.db.quote_name(s[0])
-
-    # ORDER BY clause
@@ -1343 +1396 @@
-                col_name = f
-                order = "ASC"
-
-
-
-
+
+
+
-            else:
-
-
+
+
+
+
+
+
+
-    order_by = ", ".join(order_by)
-
@@ -1364 +1421 @@
-    id_list = args and args[0] or kwargs['id_list']
-    assert id_list != [], "get_in_bulk() cannot be passed an empty list."
-opts.db_table, opts.pk.column
+db.db.quote_name(opts.db_table), db.db.quote_name(opts.pk.column)
-    kwargs['params'] = id_list
-    obj_list = function_get_list(opts, klass, **kwargs)
@@ -1385 +1442 @@
-    kwargs['order_by'] = [] # Clear this because it'll mess things up otherwise.
-    if field.null:
-
+
+
-    select, sql, params = function_get_sql_clause(opts, **kwargs)
-opts.db_table, field.column
+db.db.quote_name(opts.db_table), db.db.quote_name(field.column)
-    cursor = db.db.cursor()
-    cursor.execute(sql, params)

django/trunk/django/models/auth.py

@@ -92 +92 @@
-            import sets
-            cursor = db.cursor()
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-            self._group_perm_cache = sets.Set(["%s.%s" % (row[0], row[1]) for row in cursor.fetchall()])
-        return self._group_perm_cache

django/trunk/docs/model-api.txt

@@ -35 +35 @@
-Each field type, except for ``ForeignKey``, ``ManyToManyField`` and
-``OneToOneField``, takes an optional first positional argument -- a
-
-
-
-
+
+
+
+
+
-
-    first_name = meta.CharField("Person's first name", maxlength=30)
-
-
-
+
+
+
+
+
+
+
-
-    poll = meta.ForeignKey(Poll, verbose_name="the related poll")
@@ -111 +117 @@
-    The name of the database column to use for this field. If this isn't given,
-    Django will use the field's name.
+
+    If your database column name is an SQL reserved word, or contains
+    characters that aren't allowed in Python variable names -- notably, the
+    hyphen -- that's OK. Django quotes column and table names behind the
+    scenes.
-
-``db_index``
@@ -701 +712 @@
-    If this isn't given, Django will use ``app_label + '_' + module_name``.
-
+    If your database table name is an SQL reserved word, or contains characters
+    that aren't allowed in Python variable names -- notably, the hyphen --
+    that's OK. Django quotes column and table names behind the scenes.
+
-``exceptions``
-    Names of extra exception subclasses to include in the generated module.
@@ -733 +748 @@
-
-    If this isn't given, Django will use a lowercased version of the class
-"s". This "poor man's pluralization" is intentional: Any other
-
+``"s"``. This "poor man's pluralization" is intentional: Any
+other 
-
-``order_with_respect_to``

django/trunk/tests/testapp/models/__init__.py

@@ -2 +2 @@
-           'ordering', 'lookup', 'get_latest', 'm2m_intermediary', 'one_to_one',
-           'm2o_recursive', 'm2o_recursive2', 'save_delete_hooks', 'custom_pk',
-
+, 'reserved_names'