From 5ea1d673151a2a94b41d3131345464dfddaea95b Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Fri, 29 May 2009 18:56:50 +0000
Subject: [PATCH] - sql     - Removed an obscure feature of execute()
 (including connection,       engine, Session) whereby a bindparam() construct
 can be sent as       a key to the params dictionary.  This usage is
 undocumented       and is at the core of an issue whereby the bindparam()
 object       created implicitly by a text() construct may have the same      
 hash value as a string placed in the params dictionary and       may result
 in an inappropriate match when computing the final       bind parameters.  
 Internal checks for this condition would       add significant latency to the
 critical task of parameter       rendering, so the behavior is removed.  This
 is a backwards       incompatible change for any application that may have
 been       using this feature, however the feature has never been      
 documented.

---
 CHANGES                          | 15 +++++++++++++++
 lib/sqlalchemy/sql/compiler.py   |  2 +-
 lib/sqlalchemy/sql/expression.py |  2 +-
 test/sql/query.py                | 15 ---------------
 test/sql/select.py               | 16 +++++++++++++++-
 5 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/CHANGES b/CHANGES
index 4f47bfe09f..94a0acf88a 100644
--- a/CHANGES
+++ b/CHANGES
@@ -9,6 +9,21 @@ CHANGES
 - orm
     - Fixed bug introduced in 0.5.4 whereby Composite types
       fail when default-holding columns are flushed.
+
+- sql
+    - Removed an obscure feature of execute() (including connection,
+      engine, Session) whereby a bindparam() construct can be sent as 
+      a key to the params dictionary.  This usage is undocumented
+      and is at the core of an issue whereby the bindparam() object
+      created implicitly by a text() construct may have the same 
+      hash value as a string placed in the params dictionary and
+      may result in an inappropriate match when computing the final
+      bind parameters.   Internal checks for this condition would
+      add significant latency to the critical task of parameter 
+      rendering, so the behavior is removed.  This is a backwards
+      incompatible change for any application that may have been
+      using this feature, however the feature has never been
+      documented.
     
 0.5.4p2
 =======
diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
index 84b0ff6283..5140b7f20e 100644
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -226,7 +226,7 @@ class DefaultCompiler(engine.Compiled):
             params = util.column_dict(params)
             pd = {}
             for bindparam, name in self.bind_names.iteritems():
-                for paramname in (bindparam, bindparam.key, bindparam.shortname, name):
+                for paramname in (bindparam.key, bindparam.shortname, name):
                     if paramname in params:
                         pd[name] = params[paramname]
                         break
diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py
index 65c3c2135d..565c3407b7 100644
--- a/lib/sqlalchemy/sql/expression.py
+++ b/lib/sqlalchemy/sql/expression.py
@@ -2077,7 +2077,7 @@ class _TextClause(ClauseElement):
         if bindparams is not None:
             for b in bindparams:
                 self.bindparams[b.key] = b
-
+                
     @property
     def type(self):
         if self.typemap is not None and len(self.typemap) == 1:
diff --git a/test/sql/query.py b/test/sql/query.py
index a6c4196561..b428d8991c 100644
--- a/test/sql/query.py
+++ b/test/sql/query.py
@@ -307,21 +307,6 @@ class QueryTest(TestBase):
         r = s.execute(userid='fred').fetchall()
         assert len(r) == 1
 
-        u = bindparam('userid', unique=True)
-        s = users.select(and_(users.c.user_name==u, users.c.user_name==u))
-        r = s.execute({u:'fred'}).fetchall()
-        assert len(r) == 1
-
-    def test_bindparams_in_params(self):
-        """test that a _BindParamClause itself can be a key in the params dict"""
-
-        users.insert().execute(user_id = 7, user_name = 'jack')
-        users.insert().execute(user_id = 8, user_name = 'fred')
-
-        u = bindparam('userid')
-        r = users.select(users.c.user_name==u).execute({u:'fred'}).fetchall()
-        assert len(r) == 1
-
     def test_bindparam_shortname(self):
         """test the 'shortname' field on BindParamClause."""
         users.insert().execute(user_id = 7, user_name = 'jack')
diff --git a/test/sql/select.py b/test/sql/select.py
index 52c382f817..2ec5b8da51 100644
--- a/test/sql/select.py
+++ b/test/sql/select.py
@@ -1154,7 +1154,21 @@ UNION SELECT mytable.myid FROM mytable"
         s = select([table1], or_(table1.c.myid==7, table1.c.myid==8, table1.c.myid==bindparam('myid_1')))
         self.assertRaisesMessage(exc.CompileError, "conflicts with unique bind parameter of the same name", str, s)
 
-
+    def test_binds_no_hash_collision(self):
+        """test that construct_params doesn't corrupt dict due to hash collisions"""
+        
+        total_params = 100000
+        
+        in_clause = [':in%d' % i for i in range(total_params)]
+        params = dict(('in%d' % i, i) for i in range(total_params))
+        sql = 'text clause %s' % ', '.join(in_clause)
+        t = text(sql)
+        assert len(t.bindparams) == total_params
+        c = t.compile()
+        pp = c.construct_params(params)
+        assert len(set(pp)) == total_params
+        assert len(set(pp.values())) == total_params
+        
 
     def test_bind_as_col(self):
         t = table('foo', column('id'))
-- 
2.47.2