Re: [sqlalchemy] [PATCH] MATCH clause implementation for foreign keys
On Jun 9, 2012, at 10:41 PM, Michael Bayer wrote: it looks great. This is in the queue as http://www.sqlalchemy.org/trac/ticket/2502. 1765text += MATCH %s % constraint.match SQL injection? Shouldn't the argument be one of three constants? I suspect there needs to be some specific per-database-driver logic to handle unimplemented cases. PostgreSQL, for example, doesn't support MATCH PARTIAL ( http://www.postgresql.org/docs/9.1/static/sql-createtable.html ) and MySQL, naturally, completely ignores the syntax and triggers other clauses to be ignored: For users familiar with the ANSI/ISO SQL Standard, please note that no storage engine, including InnoDB, recognizes or enforces the MATCH clause used in referential integrity constraint definitions. Use of an explicit MATCH clause will not have the specified effect, and also causes ON DELETE and ON UPDATE clauses to be ignored. For these reasons, specifying MATCH should be avoided. http://dev.mysql.com/doc/refman/5.5/en/create-table.html Cheers, M -- You received this message because you are subscribed to the Google Groups sqlalchemy group. To post to this group, send email to sqlalchemy@googlegroups.com. To unsubscribe from this group, send email to sqlalchemy+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/sqlalchemy?hl=en.
Re: [sqlalchemy] [PATCH] MATCH clause implementation for foreign keys
On Sun, Jun 10, 2012 at 12:43:31PM -0400, A.M. wrote: On Jun 9, 2012, at 10:41 PM, Michael Bayer wrote: it looks great. This is in the queue as http://www.sqlalchemy.org/trac/ticket/2502. 1765 text += MATCH %s % constraint.match SQL injection? Shouldn't the argument be one of three constants? ON UPDATE, ON DELETE, DEFERRABLE, and INITIALLY all work this way. If this is broken, then we should fix those, too. And there are other places, like in the dialect-specific index parameters, that do this as well. I don't agree that it's a problem, however, because if we start saying what can appear there, we're necessarily limiting ourselves to the lowest common denominator. PostgreSQL, for instance, supports SET DEFAULT in ON UPDATE/DELETE, but MySQL does not. How do we handle that case? It seems like a lot of cruft would accumulate if we start specifying which values can go in these places. I suspect there needs to be some specific per-database-driver logic to handle unimplemented cases. PostgreSQL, for example, doesn't support MATCH PARTIAL ( http://www.postgresql.org/docs/9.1/static/sql-createtable.html ) This is correct. I do not believe Oracle does either. But PostgreSQL will courteously die with an error: ERROR: MATCH PARTIAL not yet implemented and MySQL, naturally, completely ignores the syntax and triggers other clauses to be ignored: For users familiar with the ANSI/ISO SQL Standard, please note that no storage engine, including InnoDB, recognizes or enforces the MATCH clause used in referential integrity constraint definitions. Use of an explicit MATCH clause will not have the specified effect, and also causes ON DELETE and ON UPDATE clauses to be ignored. For these reasons, specifying MATCH should be avoided. http://dev.mysql.com/doc/refman/5.5/en/create-table.html I really hope someone has filed a bug report against MySQL about this. The MySQL dialect can be patched to die with a CompileError (maybe) if MATCH is used, which seems like the sanest option, given that it causes unexpected behavior. I don't think the other dialects should be given the same treatment, however, because those will presumably error out if even given MATCH. Can someone test this on other databases to confirm? Cheers, M -- You received this message because you are subscribed to the Google Groups sqlalchemy group. To post to this group, send email to sqlalchemy@googlegroups.com. To unsubscribe from this group, send email to sqlalchemy+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/sqlalchemy?hl=en. -Ryan Kelly -- You received this message because you are subscribed to the Google Groups sqlalchemy group. To post to this group, send email to sqlalchemy@googlegroups.com. To unsubscribe from this group, send email to sqlalchemy+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/sqlalchemy?hl=en.
Re: [sqlalchemy] [PATCH] MATCH clause implementation for foreign keys
On Jun 10, 2012, at 12:43 PM, A.M. wrote: On Jun 9, 2012, at 10:41 PM, Michael Bayer wrote: it looks great. This is in the queue as http://www.sqlalchemy.org/trac/ticket/2502. 1765 text += MATCH %s % constraint.match SQL injection? Shouldn't the argument be one of three constants? This is not a SQL injection vector, for several reasons: 1. its a SQL keyword, not a literal value. Most database client APIs only consider literal values, not SQL keywords or schema identifiers, as candidates for bound values. While some DBAPIs, notably mysql-python, don't make use of real bound parameters and instead use Python string interpolation, technically they're doing the wrong thing. This ties into typical prepared statement mechanics which allow the database to separate the lexical aspect of a SQL statement from the data values. 2. not only is it a SQL keyword, its a keyword that's only used at DDL time. SQL injection IMO is not an issue for DDL - no sane application should ever be producing DDL at application run time, as the schema is part of the application's structure. 3. SQLAlchemy accepts textual SQL statements in many ways.The developer is not absolved from basic practices, just like putting user input into eval() should never be done, nor should user input be placed into the lexical portions of SQL statements. Bound values have special meaning within a statement which is where library-level interaction is warranted. In SQLAlchemy, we use literal strings for these values, such as for ON DELETE, ON UPDATE, WITH LOCKMODE, SQL hints, etc., so that the full array of platform specific values, including new ones that come out in newer releases, can be used without SQLAlchemy requiring modification. All of that said, all these SQL keywords can be tested against a basic sanity check, such as one that tests for alphanumeric characters only, so I wouldn't oppose a standard sanity checker for all the places we accept SQL keywords. I suspect there needs to be some specific per-database-driver logic to handle unimplemented cases. PostgreSQL, for example, doesn't support MATCH PARTIAL ( http://www.postgresql.org/docs/9.1/static/sql-createtable.html ) and MySQL, naturally, completely ignores the syntax and triggers other clauses to be ignored: SQLAlchemy's current technique for this common situation is to use conditional DDL techniques: http://docs.sqlalchemy.org/en/rel_0_7/core/schema.html#sqlalchemy.schema.DDLElement.execute_if http://docs.sqlalchemy.org/en/rel_0_7/core/schema.html#metadata-ddl -- You received this message because you are subscribed to the Google Groups sqlalchemy group. To post to this group, send email to sqlalchemy@googlegroups.com. To unsubscribe from this group, send email to sqlalchemy+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/sqlalchemy?hl=en.
[sqlalchemy] [PATCH] MATCH clause implementation for foreign keys
All: The attached patch implements the MATCH keyword for foreign keys. This is part of the SQL92 standard, and is supported by PostgreSQL and Oracle. Feedback welcome. -Ryan Kelly -- You received this message because you are subscribed to the Google Groups sqlalchemy group. To post to this group, send email to sqlalchemy@googlegroups.com. To unsubscribe from this group, send email to sqlalchemy+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/sqlalchemy?hl=en. diff -r ea4bd6b54789 lib/sqlalchemy/schema.py --- a/lib/sqlalchemy/schema.py Fri Jun 08 15:56:58 2012 -0400 +++ b/lib/sqlalchemy/schema.py Sat Jun 09 19:02:42 2012 -0400 @@ -1191,7 +1191,7 @@ def __init__(self, column, _constraint=None, use_alter=False, name=None, onupdate=None, ondelete=None, deferrable=None, schema=None, -initially=None, link_to_name=False): +initially=None, link_to_name=False, match=None): Construct a column-level FOREIGN KEY. @@ -1236,6 +1236,10 @@ generated/dropped externally from the CREATE TABLE/ DROP TABLE statement. See that classes' constructor for details. +:param match: Optional string. If set, emit MATCH value when issuing +DDL for this constraint. Typical values include SIMPLE, PARTIAL +and FULL. + self._colspec = column @@ -1255,6 +1259,7 @@ self.deferrable = deferrable self.initially = initially self.link_to_name = link_to_name +self.match = match def __repr__(self): return ForeignKey(%r) % self._get_colspec() @@ -1283,7 +1288,8 @@ ondelete=self.ondelete, deferrable=self.deferrable, initially=self.initially, -link_to_name=self.link_to_name +link_to_name=self.link_to_name, +match=self.match ) fk.dispatch._update(self.dispatch) return fk @@ -1445,6 +1451,7 @@ [], [], use_alter=self.use_alter, name=self.name, onupdate=self.onupdate, ondelete=self.ondelete, deferrable=self.deferrable, initially=self.initially, +match=self.match, ) self.constraint._elements[self.parent] = self self.constraint._set_parent_with_dispatch(table) @@ -2031,7 +2038,7 @@ def __init__(self, columns, refcolumns, name=None, onupdate=None, ondelete=None, deferrable=None, initially=None, use_alter=False, -link_to_name=False, table=None): +link_to_name=False, match=None, table=None): Construct a composite-capable FOREIGN KEY. :param columns: A sequence of local column names. The named columns @@ -2072,6 +2079,10 @@ This is normally used to generate/drop constraints on objects that are mutually dependent on each other. +:param match: Optional string. If set, emit MATCH value when issuing +DDL for this constraint. Typical values include SIMPLE, PARTIAL +and FULL. + super(ForeignKeyConstraint, self).\ __init__(name, deferrable, initially) @@ -2082,6 +2093,7 @@ if self.name is None and use_alter: raise exc.ArgumentError(Alterable Constraint requires a name) self.use_alter = use_alter +self.match = match self._elements = util.OrderedDict() @@ -2097,7 +2109,8 @@ onupdate=self.onupdate, ondelete=self.ondelete, use_alter=self.use_alter, -link_to_name=self.link_to_name +link_to_name=self.link_to_name, +match=self.match ) if table is not None: @@ -2153,7 +2166,8 @@ use_alter=self.use_alter, deferrable=self.deferrable, initially=self.initially, -link_to_name=self.link_to_name +link_to_name=self.link_to_name, +match=self.match ) fkc.dispatch._update(self.dispatch) return fkc diff -r ea4bd6b54789 lib/sqlalchemy/sql/compiler.py --- a/lib/sqlalchemy/sql/compiler.py Fri Jun 08 15:56:58 2012 -0400 +++ b/lib/sqlalchemy/sql/compiler.py Sat Jun 09 19:02:42 2012 -0400 @@ -1719,6 +1719,7 @@ ', '.join(preparer.quote(f.column.name, f.column.quote) for f in constraint._elements.values()) ) +text += self.define_constraint_match(constraint) text += self.define_constraint_cascades(constraint) text += self.define_constraint_deferrability(constraint) return text @@ -1758,6 +1759,12 @@ text += INITIALLY %s % constraint.initially
Re: [sqlalchemy] [PATCH] MATCH clause implementation for foreign keys
it looks great. This is in the queue as http://www.sqlalchemy.org/trac/ticket/2502. On Jun 9, 2012, at 7:15 PM, Ryan Kelly wrote: All: The attached patch implements the MATCH keyword for foreign keys. This is part of the SQL92 standard, and is supported by PostgreSQL and Oracle. Feedback welcome. -Ryan Kelly -- You received this message because you are subscribed to the Google Groups sqlalchemy group. To post to this group, send email to sqlalchemy@googlegroups.com. To unsubscribe from this group, send email to sqlalchemy+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/sqlalchemy?hl=en. fkey_match.patch -- You received this message because you are subscribed to the Google Groups sqlalchemy group. To post to this group, send email to sqlalchemy@googlegroups.com. To unsubscribe from this group, send email to sqlalchemy+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/sqlalchemy?hl=en.
