- Fixed bug where the truncation of long labels in SQL could produce

a label that overlapped another label that is not truncated; this
because the length threshhold for truncation was greater than
the portion of the label that remains after truncation.  These
two values have now been made the same; label_length - 6.
The effect here is that shorter column labels will be "truncated"
where they would not have been truncated before.
fixes #3396

3 files changed, 58 insertions, 23 deletions

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index 5b0ddf64f..5c39566b9 100644
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -19,6 +19,18 @@
     :version: 1.0.3
 
     .. change::
+        :tags: bug, sql
+        :tickets: 3396
+
+        Fixed bug where the truncation of long labels in SQL could produce
+        a label that overlapped another label that is not truncated; this
+        because the length threshhold for truncation was greater than
+        the portion of the label that remains after truncation.  These
+        two values have now been made the same; label_length - 6.
+        The effect here is that shorter column labels will be "truncated"
+        where they would not have been truncated before.
+
+    .. change::
         :tags: bug, orm
         :tickets: 3392
 
diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
index 91b677a0e..c9c7fd2a1 100644
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -1133,7 +1133,7 @@ class SQLCompiler(Compiled):
 
         anonname = name.apply_map(self.anon_map)
 
-        if len(anonname) > self.label_length:
+        if len(anonname) > self.label_length - 6:
             counter = self.truncated_names.get(ident_class, 1)
             truncname = anonname[0:max(self.label_length - 6, 0)] + \
                 "_" + hex(counter)[2:]
diff --git a/test/sql/test_labels.py b/test/sql/test_labels.py
index 1792a42d8..7f548eb49 100644
--- a/test/sql/test_labels.py
+++ b/test/sql/test_labels.py
@@ -138,8 +138,9 @@ class MaxIdentTest(fixtures.TestBase, AssertsCompiledSQL):
             issuperset(['this_is_the_data_column',
                         s.c.this_is_the_data_column])
         assert \
-            set(compiled._create_result_map()['this_is_the_primarykey_column'][1]).\
+            set(compiled._create_result_map()['this_is_the_primarykey__1'][1]).\
             issuperset(['this_is_the_primarykey_column',
+                        'this_is_the_primarykey__1',
                         s.c.this_is_the_primarykey_column])
 
     def test_result_map_anon_alias(self):
@@ -150,29 +151,28 @@ class MaxIdentTest(fixtures.TestBase, AssertsCompiledSQL):
         s = select([q]).apply_labels()
 
         self.assert_compile(
-            s, 'SELECT '
-            'anon_1.this_is_the_primarykey_column '
-            'AS anon_1_this_is_the_prim_1, '
-            'anon_1.this_is_the_data_column '
-            'AS anon_1_this_is_the_data_2 '
-            'FROM ('
-            'SELECT '
-            'some_large_named_table.'
-            'this_is_the_primarykey_column '
-            'AS this_is_the_primarykey_column, '
-            'some_large_named_table.this_is_the_data_column '
-            'AS this_is_the_data_column '
-            'FROM '
-            'some_large_named_table '
-            'WHERE '
-            'some_large_named_table.this_is_the_primarykey_column '
-            '= :this_is_the_primarykey__1'
-            ') '
-            'AS anon_1', dialect=dialect)
+            s,
+            "SELECT "
+            "anon_1.this_is_the_primarykey__2 AS anon_1_this_is_the_prim_1, "
+            "anon_1.this_is_the_data_column AS anon_1_this_is_the_data_3 "
+            "FROM ("
+            "SELECT "
+            "some_large_named_table."
+            "this_is_the_primarykey_column AS this_is_the_primarykey__2, "
+            "some_large_named_table."
+            "this_is_the_data_column AS this_is_the_data_column "
+            "FROM "
+            "some_large_named_table "
+            "WHERE "
+            "some_large_named_table.this_is_the_primarykey_column "
+            "= :this_is_the_primarykey__1"
+            ") "
+            "AS anon_1", dialect=dialect)
+
         compiled = s.compile(dialect=dialect)
-        assert set(compiled._create_result_map()['anon_1_this_is_the_data_2'][1]).\
+        assert set(compiled._create_result_map()['anon_1_this_is_the_data_3'][1]).\
             issuperset([
-                'anon_1_this_is_the_data_2',
+                'anon_1_this_is_the_data_3',
                 q.corresponding_column(
                     table1.c.this_is_the_data_column)
             ])
@@ -542,3 +542,26 @@ class LabelLengthTest(fixtures.TestBase, AssertsCompiledSQL):
         compiled = s.compile(dialect=dialect)
         assert set(compiled._create_result_map()['_1'][1]).issuperset([
             'asdf_abcde', a1.c.abcde, '_1'])
+
+    def test_label_overlap_unlabeled(self):
+        """test that an anon col can't overlap with a fixed name, #3396"""
+
+        table1 = table(
+            "tablename", column('columnname_one'), column('columnn_1'))
+
+        stmt = select([table1]).apply_labels()
+
+        dialect = default.DefaultDialect(label_length=23)
+        self.assert_compile(
+            stmt,
+            "SELECT tablename.columnname_one AS tablename_columnn_1, "
+            "tablename.columnn_1 AS tablename_columnn_2 FROM tablename",
+            dialect=dialect
+        )
+        compiled = stmt.compile(dialect=dialect)
+        eq_(
+            set(compiled._create_result_map()),
+            set(['tablename_columnn_1', 'tablename_columnn_2'])
+        )
+
+