diff options
| -rw-r--r-- | doc/build/changelog/changelog_10.rst | 18 | ||||
| -rw-r--r-- | lib/sqlalchemy/sql/compiler.py | 21 | ||||
| -rw-r--r-- | lib/sqlalchemy/sql/elements.py | 24 | ||||
| -rw-r--r-- | lib/sqlalchemy/sql/selectable.py | 3 | ||||
| -rw-r--r-- | test/sql/test_compiler.py | 6 | ||||
| -rw-r--r-- | test/sql/test_selectable.py | 31 |
6 files changed, 87 insertions, 16 deletions
diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst index 859adae81..52b35c931 100644 --- a/doc/build/changelog/changelog_10.rst +++ b/doc/build/changelog/changelog_10.rst @@ -19,6 +19,24 @@ :version: 1.0.1 .. change:: + :tags: bug, core + :tickets: 3372 + + Fixed issue where a straight SELECT EXISTS query would fail to + assign the proper result type of Boolean to the result mapping, and + instead would leak column types from within the query into the + result map. This issue exists in 0.9 and earlier as well, however + has less of an impact in those versions. In 1.0, due to #918 + this becomes a regression in that we now rely upon the result mapping + to be very accurate, else we can assign result-type processors to + the wrong column. In all versions, this issue also has the effect + that a simple EXISTS will not apply the Boolean type handler, leading + to simple 1/0 values for backends without native boolean instead of + True/False. The fix includes that an EXISTS columns argument + will be anon-labeled like other column expressions; a similar fix is + implemented for pure-boolean expressions like ``not_(True())``. + + .. change:: :tags: bug, orm :tickets: 3374 diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index 755193552..5633159cd 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -1324,10 +1324,17 @@ class SQLCompiler(Compiled): result_expr = _CompileLabel(col_expr, elements._as_truncated(column.name), alt_names=(column.key,)) - elif not isinstance(column, - (elements.UnaryExpression, elements.TextClause)) \ - and (not hasattr(column, 'name') or - isinstance(column, functions.Function)): + elif ( + not isinstance(column, elements.TextClause) and + ( + not isinstance(column, elements.UnaryExpression) or + column.wraps_column_expression + ) and + ( + not hasattr(column, 'name') or + isinstance(column, functions.Function) + ) + ): result_expr = _CompileLabel(col_expr, column.anon_label) elif col_expr is not column: # TODO: are we sure "column" has a .name and .key here ? @@ -1528,6 +1535,12 @@ class SQLCompiler(Compiled): 'need_result_map_for_compound', False) ) or entry.get('need_result_map_for_nested', False) + # this was first proposed as part of #3372; however, it is not + # reached in current tests and could possibly be an assertion + # instead. + if not populate_result_map and 'add_to_result_map' in kwargs: + del kwargs['add_to_result_map'] + if needs_nested_translation: if populate_result_map: self._transform_result_map_for_nested_joins( diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py index ca8ec1f55..6ee4053a7 100644 --- a/lib/sqlalchemy/sql/elements.py +++ b/lib/sqlalchemy/sql/elements.py @@ -2407,13 +2407,14 @@ class UnaryExpression(ColumnElement): __visit_name__ = 'unary' def __init__(self, element, operator=None, modifier=None, - type_=None, negate=None): + type_=None, negate=None, wraps_column_expression=False): self.operator = operator self.modifier = modifier self.element = element.self_group( against=self.operator or self.modifier) self.type = type_api.to_instance(type_) self.negate = negate + self.wraps_column_expression = wraps_column_expression @classmethod def _create_nullsfirst(cls, column): @@ -2455,7 +2456,8 @@ class UnaryExpression(ColumnElement): """ return UnaryExpression( _literal_as_label_reference(column), - modifier=operators.nullsfirst_op) + modifier=operators.nullsfirst_op, + wraps_column_expression=False) @classmethod def _create_nullslast(cls, column): @@ -2496,7 +2498,8 @@ class UnaryExpression(ColumnElement): """ return UnaryExpression( _literal_as_label_reference(column), - modifier=operators.nullslast_op) + modifier=operators.nullslast_op, + wraps_column_expression=False) @classmethod def _create_desc(cls, column): @@ -2534,7 +2537,9 @@ class UnaryExpression(ColumnElement): """ return UnaryExpression( - _literal_as_label_reference(column), modifier=operators.desc_op) + _literal_as_label_reference(column), + modifier=operators.desc_op, + wraps_column_expression=False) @classmethod def _create_asc(cls, column): @@ -2571,7 +2576,9 @@ class UnaryExpression(ColumnElement): """ return UnaryExpression( - _literal_as_label_reference(column), modifier=operators.asc_op) + _literal_as_label_reference(column), + modifier=operators.asc_op, + wraps_column_expression=False) @classmethod def _create_distinct(cls, expr): @@ -2611,7 +2618,8 @@ class UnaryExpression(ColumnElement): """ expr = _literal_as_binds(expr) return UnaryExpression( - expr, operator=operators.distinct_op, type_=expr.type) + expr, operator=operators.distinct_op, + type_=expr.type, wraps_column_expression=False) @property def _order_by_label_element(self): @@ -2648,7 +2656,8 @@ class UnaryExpression(ColumnElement): operator=self.negate, negate=self.operator, modifier=self.modifier, - type_=self.type) + type_=self.type, + wraps_column_expression=self.wraps_column_expression) else: return ClauseElement._negate(self) @@ -2667,6 +2676,7 @@ class AsBoolean(UnaryExpression): self.operator = operator self.negate = negate self.modifier = None + self.wraps_column_expression = True def self_group(self, against=None): return self diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py index f848ef6db..7d8c885ae 100644 --- a/lib/sqlalchemy/sql/selectable.py +++ b/lib/sqlalchemy/sql/selectable.py @@ -3343,7 +3343,8 @@ class Exists(UnaryExpression): s = Select(*args, **kwargs).as_scalar().self_group() UnaryExpression.__init__(self, s, operator=operators.exists, - type_=type_api.BOOLEANTYPE) + type_=type_api.BOOLEANTYPE, + wraps_column_expression=True) def select(self, whereclause=None, **params): return Select([self], whereclause, **params) diff --git a/test/sql/test_compiler.py b/test/sql/test_compiler.py index 4bdd9fcdc..75f9a7c82 100644 --- a/test/sql/test_compiler.py +++ b/test/sql/test_compiler.py @@ -380,7 +380,7 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL): # this is native_boolean=False for default dialect self.assert_compile( select([not_(True)], use_labels=True), - "SELECT :param_1 = 0" + "SELECT :param_1 = 0 AS anon_1" ) self.assert_compile( @@ -561,13 +561,13 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL): self.assert_compile(exists([table1.c.myid], table1.c.myid == 5).select(), 'SELECT EXISTS (SELECT mytable.myid FROM ' - 'mytable WHERE mytable.myid = :myid_1)', + 'mytable WHERE mytable.myid = :myid_1) AS anon_1', params={'mytable_myid': 5}) self.assert_compile(select([table1, exists([1], from_obj=table2)]), 'SELECT mytable.myid, mytable.name, ' 'mytable.description, EXISTS (SELECT 1 ' - 'FROM myothertable) FROM mytable', + 'FROM myothertable) AS anon_1 FROM mytable', params={}) self.assert_compile(select([table1, exists([1], diff --git a/test/sql/test_selectable.py b/test/sql/test_selectable.py index 3931f99e4..3390f4a77 100644 --- a/test/sql/test_selectable.py +++ b/test/sql/test_selectable.py @@ -2113,7 +2113,7 @@ class WithLabelsTest(fixtures.TestBase): self._assert_result_keys(sel, ['t1_a', 't2_b']) -class SelectProxyTest(fixtures.TestBase): +class ResultMapTest(fixtures.TestBase): def _fixture(self): m = MetaData() @@ -2183,6 +2183,35 @@ class SelectProxyTest(fixtures.TestBase): assert l1 in mapping assert ta.c.x not in mapping + def test_column_subquery_exists(self): + t = self._fixture() + s = exists().where(t.c.x == 5).select() + mapping = self._mapping(s) + assert t.c.x not in mapping + eq_( + [type(entry[-1]) for entry in s.compile()._result_columns], + [Boolean] + ) + + def test_column_subquery_plain(self): + t = self._fixture() + s1 = select([t.c.x]).where(t.c.x > 5).as_scalar() + s2 = select([s1]) + mapping = self._mapping(s2) + assert t.c.x not in mapping + assert s1 in mapping + eq_( + [type(entry[-1]) for entry in s2.compile()._result_columns], + [Integer] + ) + + def test_unary_boolean(self): + + s1 = select([not_(True)], use_labels=True) + eq_( + [type(entry[-1]) for entry in s1.compile()._result_columns], + [Boolean] + ) class ForUpdateTest(fixtures.TestBase, AssertsCompiledSQL): __dialect__ = "default" |