From ac52239b328f6dc573fdfb9acbbc7d5d528fa982 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Tue, 28 Apr 2015 16:02:59 -0400 Subject: [PATCH] - 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 --- doc/build/changelog/changelog_10.rst | 12 +++++ lib/sqlalchemy/sql/compiler.py | 2 +- test/sql/test_labels.py | 67 +++++++++++++++++++--------- 3 files changed, 58 insertions(+), 23 deletions(-) diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst index 5b0ddf64fe..5c39566b90 100644 --- a/doc/build/changelog/changelog_10.rst +++ b/doc/build/changelog/changelog_10.rst @@ -18,6 +18,18 @@ .. changelog:: :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 91b677a0ef..c9c7fd2a15 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 1792a42d8f..7f548eb491 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']) + ) + + -- 2.47.3