From: Mike Bayer Date: Sat, 15 Aug 2020 19:08:09 +0000 (-0400) Subject: Create a real type for Tuple() and handle appropriately in compiler X-Git-Tag: rel_1_4_0b1~167^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3b4bbbb2a3d337d0af1ba5ccb0d29d1c48735e83;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Create a real type for Tuple() and handle appropriately in compiler Improved the :func:`_sql.tuple_` construct such that it behaves predictably when used in a columns-clause context. The SQL tuple is not supported as a "SELECT" columns clause element on most backends; on those that do (PostgreSQL, not surprisingly), the Python DBAPI does not have a "nested type" concept so there are still challenges in fetching rows for such an object. Use of :func:`_sql.tuple_` in a :func:`_sql.select` or :class:`_orm.Query` will now raise a :class:`_exc.CompileError` at the point at which the :func:`_sql.tuple_` object is seen as presenting itself for fetching rows (i.e., if the tuple is in the columns clause of a subquery, no error is raised). For ORM use,the :class:`_orm.Bundle` object is an explicit directive that a series of columns should be returned as a sub-tuple per row and is suggested by the error message. Additionally ,the tuple will now render with parenthesis in all contexts. Previously, the parenthesization would not render in a columns context leading to non-defined behavior. As part of this change, Tuple receives a dedicated datatype which appears to allow us the very desirable change of removing the bindparam._expanding_in_types attribute as well as ClauseList._tuple_values (which might already have not been needed due to #4645). Fixes: #5127 Change-Id: Iecafa0e0aac2f1f37ec8d0e1631d562611c90200 --- diff --git a/doc/build/changelog/unreleased_14/5127.rst b/doc/build/changelog/unreleased_14/5127.rst new file mode 100644 index 0000000000..94e5846490 --- /dev/null +++ b/doc/build/changelog/unreleased_14/5127.rst @@ -0,0 +1,19 @@ +.. change:: + :tags: bug, sql + :tickets: 5127 + + Improved the :func:`_sql.tuple_` construct such that it behaves predictably + when used in a columns-clause context. The SQL tuple is not supported as a + "SELECT" columns clause element on most backends; on those that do + (PostgreSQL, not surprisingly), the Python DBAPI does not have a "nested + type" concept so there are still challenges in fetching rows for such an + object. Use of :func:`_sql.tuple_` in a :func:`_sql.select` or + :class:`_orm.Query` will now raise a :class:`_exc.CompileError` at the + point at which the :func:`_sql.tuple_` object is seen as presenting itself + for fetching rows (i.e., if the tuple is in the columns clause of a + subquery, no error is raised). For ORM use,the :class:`_orm.Bundle` object + is an explicit directive that a series of columns should be returned as a + sub-tuple per row and is suggested by the error message. Additionally ,the + tuple will now render with parenthesis in all contexts. Previously, the + parenthesization would not render in a columns context leading to + non-defined behavior. diff --git a/lib/sqlalchemy/dialects/postgresql/array.py b/lib/sqlalchemy/dialects/postgresql/array.py index 84fbd2e501..7fd271d523 100644 --- a/lib/sqlalchemy/dialects/postgresql/array.py +++ b/lib/sqlalchemy/dialects/postgresql/array.py @@ -9,8 +9,10 @@ import re from ... import types as sqltypes from ... import util +from ...sql import coercions from ...sql import expression from ...sql import operators +from ...sql import roles def Any(other, arrexpr, operator=operators.eq): @@ -41,7 +43,7 @@ def All(other, arrexpr, operator=operators.eq): return arrexpr.all(other, operator) -class array(expression.Tuple): +class array(expression.ClauseList, expression.ColumnElement): """A PostgreSQL ARRAY literal. @@ -97,16 +99,31 @@ class array(expression.Tuple): __visit_name__ = "array" def __init__(self, clauses, **kw): + clauses = [ + coercions.expect(roles.ExpressionElementRole, c) for c in clauses + ] + super(array, self).__init__(*clauses, **kw) - if isinstance(self.type, ARRAY): + + self._type_tuple = [arg.type for arg in clauses] + main_type = kw.pop( + "type_", + self._type_tuple[0] if self._type_tuple else sqltypes.NULLTYPE, + ) + + if isinstance(main_type, ARRAY): self.type = ARRAY( - self.type.item_type, - dimensions=self.type.dimensions + 1 - if self.type.dimensions is not None + main_type.item_type, + dimensions=main_type.dimensions + 1 + if main_type.dimensions is not None else 2, ) else: - self.type = ARRAY(self.type) + self.type = ARRAY(main_type) + + @property + def _select_iterable(self): + return (self,) def _bind_param(self, operator, obj, _assume_scalar=False, type_=None): if _assume_scalar or operator is operators.getitem: diff --git a/lib/sqlalchemy/engine/default.py b/lib/sqlalchemy/engine/default.py index 4fb20a3d50..ec0f2ed9f0 100644 --- a/lib/sqlalchemy/engine/default.py +++ b/lib/sqlalchemy/engine/default.py @@ -1456,8 +1456,8 @@ class DefaultExecutionContext(interfaces.ExecutionContext): continue if key in self._expanded_parameters: - if bindparam._expanding_in_types: - num = len(bindparam._expanding_in_types) + if bindparam.type._is_tuple_type: + num = len(bindparam.type.types) dbtypes = inputsizes[bindparam] positional_inputsizes.extend( [ @@ -1488,8 +1488,8 @@ class DefaultExecutionContext(interfaces.ExecutionContext): continue if key in self._expanded_parameters: - if bindparam._expanding_in_types: - num = len(bindparam._expanding_in_types) + if bindparam.type._is_tuple_type: + num = len(bindparam.type.types) dbtypes = inputsizes[bindparam] keyword_inputsizes.update( [ diff --git a/lib/sqlalchemy/orm/evaluator.py b/lib/sqlalchemy/orm/evaluator.py index caa9ffe108..21d5e72d40 100644 --- a/lib/sqlalchemy/orm/evaluator.py +++ b/lib/sqlalchemy/orm/evaluator.py @@ -113,6 +113,9 @@ class EvaluatorCompiler(object): get_corresponding_attr = operator.attrgetter(key) return lambda obj: get_corresponding_attr(obj) + def visit_tuple(self, clause): + return self.visit_clauselist(clause) + def visit_clauselist(self, clause): evaluators = list(map(self.process, clause.clauses)) if clause.operator is operators.or_: diff --git a/lib/sqlalchemy/sql/coercions.py b/lib/sqlalchemy/sql/coercions.py index fa0f9c4357..b3a38f8024 100644 --- a/lib/sqlalchemy/sql/coercions.py +++ b/lib/sqlalchemy/sql/coercions.py @@ -478,7 +478,6 @@ class InElementImpl(RoleImpl): if non_literal_expressions: return elements.ClauseList( - _tuple_values=isinstance(expr, elements.Tuple), *[ non_literal_expressions[o] if o in non_literal_expressions @@ -509,10 +508,6 @@ class InElementImpl(RoleImpl): # param to IN? check for ARRAY type? element = element._clone(maintain_key=True) element.expanding = True - if isinstance(expr, elements.Tuple): - element = element._with_expanding_in_types( - [elem.type for elem in expr] - ) return element else: diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index 542bf58ac5..4f4cf7f8b9 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -848,10 +848,10 @@ class SQLCompiler(Compiled): ( self.bind_names[bindparam], bindparam.type._cached_bind_processor(self.dialect) - if not bindparam._expanding_in_types + if not bindparam.type._is_tuple_type else tuple( elem_type._cached_bind_processor(self.dialect) - for elem_type in bindparam._expanding_in_types + for elem_type in bindparam.type.types ), ) for bindparam in self.bind_names @@ -1018,9 +1018,9 @@ class SQLCompiler(Compiled): if bindparam in literal_execute_params: continue - if bindparam._expanding_in_types: + if bindparam.type._is_tuple_type: inputsizes[bindparam] = [ - _lookup_type(typ) for typ in bindparam._expanding_in_types + _lookup_type(typ) for typ in bindparam.type.types ] else: inputsizes[bindparam] = _lookup_type(bindparam.type) @@ -1107,7 +1107,7 @@ class SQLCompiler(Compiled): if not parameter.literal_execute: parameters.update(to_update) - if parameter._expanding_in_types: + if parameter.type._is_tuple_type: new_processors.update( ( "%s_%s_%s" % (name, i, j), @@ -1541,6 +1541,9 @@ class SQLCompiler(Compiled): if s ) + def visit_tuple(self, clauselist, **kw): + return "(%s)" % self.visit_clauselist(clauselist, **kw) + def visit_clauselist(self, clauselist, **kw): sep = clauselist.operator if sep is None: @@ -1548,10 +1551,7 @@ class SQLCompiler(Compiled): else: sep = OPERATORS[clauselist.operator] - text = self._generate_delimited_list(clauselist.clauses, sep, **kw) - if clauselist._tuple_values and self.dialect.tuple_in_values: - text = "VALUES " + text - return text + return self._generate_delimited_list(clauselist.clauses, sep, **kw) def visit_case(self, clause, **kwargs): x = "CASE " @@ -1824,27 +1824,31 @@ class SQLCompiler(Compiled): def _literal_execute_expanding_parameter_literal_binds( self, parameter, values ): + if not values: + assert not parameter.type._is_tuple_type replacement_expression = self.visit_empty_set_expr( - parameter._expanding_in_types - if parameter._expanding_in_types - else [parameter.type] + [parameter.type] ) elif isinstance(values[0], (tuple, list)): + assert parameter.type._is_tuple_type replacement_expression = ( "VALUES " if self.dialect.tuple_in_values else "" ) + ", ".join( "(%s)" % ( ", ".join( - self.render_literal_value(value, parameter.type) - for value in tuple_element + self.render_literal_value(value, param_type) + for value, param_type in zip( + tuple_element, parameter.type.types + ) ) ) for i, tuple_element in enumerate(values) ) else: + assert not parameter.type._is_tuple_type replacement_expression = ", ".join( self.render_literal_value(value, parameter.type) for value in values @@ -1853,6 +1857,7 @@ class SQLCompiler(Compiled): return (), replacement_expression def _literal_execute_expanding_parameter(self, name, parameter, values): + if parameter.literal_execute: return self._literal_execute_expanding_parameter_literal_binds( parameter, values @@ -1860,11 +1865,15 @@ class SQLCompiler(Compiled): if not values: to_update = [] - replacement_expression = self.visit_empty_set_expr( - parameter._expanding_in_types - if parameter._expanding_in_types - else [parameter.type] - ) + if parameter.type._is_tuple_type: + + replacement_expression = self.visit_empty_set_expr( + parameter.type.types + ) + else: + replacement_expression = self.visit_empty_set_expr( + [parameter.type] + ) elif isinstance(values[0], (tuple, list)): to_update = [ @@ -2560,6 +2569,12 @@ class SQLCompiler(Compiled): if keyname is None: self._ordered_columns = False self._textual_ordered_columns = True + if type_._is_tuple_type: + raise exc.CompileError( + "Most backends don't support SELECTing " + "from a tuple() object. If this is an ORM query, " + "consider using the Bundle object." + ) self._result_columns.append((keyname, name, objects, type_)) def _label_select_column( diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py index 8a506446db..a176120340 100644 --- a/lib/sqlalchemy/sql/elements.py +++ b/lib/sqlalchemy/sql/elements.py @@ -1059,7 +1059,6 @@ class BindParameter(roles.InElementRole, ColumnElement): ] _is_crud = False - _expanding_in_types = () _is_bind_parameter = True _key_is_anon = False @@ -1372,15 +1371,6 @@ class BindParameter(roles.InElementRole, ColumnElement): else: self.type = type_ - def _with_expanding_in_types(self, types): - """Return a copy of this :class:`.BindParameter` in - the context of an expanding IN against a tuple. - - """ - cloned = self._clone(maintain_key=True) - cloned._expanding_in_types = types - return cloned - def _with_value(self, value, maintain_key=False): """Return a copy of this :class:`.BindParameter` with the given value set. @@ -2141,7 +2131,6 @@ class ClauseList( self.group_contents = kwargs.pop("group_contents", True) if kwargs.pop("_flatten_sub_clauses", False): clauses = util.flatten_iterator(clauses) - self._tuple_values = kwargs.pop("_tuple_values", False) self._text_converter_role = text_converter_role = kwargs.pop( "_literal_as_text_role", roles.WhereHavingRole ) @@ -2168,7 +2157,6 @@ class ClauseList( self.group = True self.operator = operator self.group_contents = True - self._tuple_values = False self._is_implicitly_boolean = False return self @@ -2212,8 +2200,6 @@ class BooleanClauseList(ClauseList, ColumnElement): __visit_name__ = "clauselist" inherit_cache = True - _tuple_values = False - def __init__(self, *arg, **kw): raise NotImplementedError( "BooleanClauseList has a private constructor" @@ -2471,8 +2457,11 @@ or_ = BooleanClauseList.or_ class Tuple(ClauseList, ColumnElement): """Represent a SQL tuple.""" + __visit_name__ = "tuple" + _traverse_internals = ClauseList._traverse_internals + [] + @util.preload_module("sqlalchemy.sql.sqltypes") def __init__(self, *clauses, **kw): """Return a :class:`.Tuple`. @@ -2496,15 +2485,12 @@ class Tuple(ClauseList, ColumnElement): invoked. """ + sqltypes = util.preloaded.sql_sqltypes clauses = [ coercions.expect(roles.ExpressionElementRole, c) for c in clauses ] - self._type_tuple = [arg.type for arg in clauses] - self.type = kw.pop( - "type_", - self._type_tuple[0] if self._type_tuple else type_api.NULLTYPE, - ) + self.type = sqltypes.TupleType(*[arg.type for arg in clauses]) super(Tuple, self).__init__(*clauses, **kw) @@ -2520,7 +2506,8 @@ class Tuple(ClauseList, ColumnElement): _compared_to_operator=operator, unique=True, expanding=True, - )._with_expanding_in_types(self._type_tuple) + type_=self.type, + ) else: return Tuple( *[ @@ -2532,9 +2519,13 @@ class Tuple(ClauseList, ColumnElement): unique=True, type_=type_, ) - for o, compared_to_type in zip(obj, self._type_tuple) + for o, compared_to_type in zip(obj, self.type.types) ] - ).self_group() + ) + + def self_group(self, against=None): + # Tuple is parenthsized by definition. + return self class Case(ColumnElement): diff --git a/lib/sqlalchemy/sql/sqltypes.py b/lib/sqlalchemy/sql/sqltypes.py index 64663a6b01..d29f233615 100644 --- a/lib/sqlalchemy/sql/sqltypes.py +++ b/lib/sqlalchemy/sql/sqltypes.py @@ -2801,6 +2801,21 @@ class ARRAY(SchemaEventTarget, Indexable, Concatenable, TypeEngine): self.item_type._set_parent_with_dispatch(parent) +class TupleType(TypeEngine): + """represent the composite type of a Tuple.""" + + _is_tuple_type = True + + def __init__(self, *types): + self.types = types + + def result_processor(self, dialect, coltype): + raise NotImplementedError( + "The tuple type does not support being fetched " + "as a column in a result row." + ) + + class REAL(Float): """The SQL REAL type.""" diff --git a/lib/sqlalchemy/sql/type_api.py b/lib/sqlalchemy/sql/type_api.py index 1284ef5155..0da88dc54e 100644 --- a/lib/sqlalchemy/sql/type_api.py +++ b/lib/sqlalchemy/sql/type_api.py @@ -45,6 +45,7 @@ class TypeEngine(Traversible): _sqla_type = True _isnull = False + _is_tuple_type = False class Comparator(operators.ColumnOperators): """Base class for custom comparison operations defined at the diff --git a/lib/sqlalchemy/testing/suite/test_select.py b/lib/sqlalchemy/testing/suite/test_select.py index 9fb4816767..cff1f2cfc2 100644 --- a/lib/sqlalchemy/testing/suite/test_select.py +++ b/lib/sqlalchemy/testing/suite/test_select.py @@ -665,6 +665,30 @@ class PostCompileParamsTest( ) ) + @testing.requires.tuple_in + def test_execute_tuple_expanding_plus_literal_heterogeneous_execute(self): + table = self.tables.some_table + + stmt = select([table.c.id]).where( + tuple_(table.c.x, table.c.z).in_( + bindparam("q", expanding=True, literal_execute=True) + ) + ) + + with self.sql_execution_asserter() as asserter: + with config.db.connect() as conn: + conn.execute(stmt, q=[(5, "z1"), (12, "z3")]) + + asserter.assert_( + CursorSQL( + "SELECT some_table.id \nFROM some_table " + "\nWHERE (some_table.x, some_table.z) " + "IN (%s(5, 'z1'), (12, 'z3'))" + % ("VALUES " if config.db.dialect.tuple_in_values else ""), + () if config.db.dialect.positional else {}, + ) + ) + class ExpandingBoundInTest(fixtures.TablesTest): __backend__ = True diff --git a/test/dialect/postgresql/test_dialect.py b/test/dialect/postgresql/test_dialect.py index 57c243442e..d653d04b34 100644 --- a/test/dialect/postgresql/test_dialect.py +++ b/test/dialect/postgresql/test_dialect.py @@ -828,7 +828,7 @@ $$ LANGUAGE plpgsql; def test_extract(self, connection): fivedaysago = testing.db.scalar( - select(func.now()) + select(func.now().op("at time zone")("UTC")) ) - datetime.timedelta(days=5) for field, exp in ( ("year", fivedaysago.year), @@ -837,7 +837,11 @@ $$ LANGUAGE plpgsql; ): r = connection.execute( select( - extract(field, func.now() + datetime.timedelta(days=-5)) + extract( + field, + func.now().op("at time zone")("UTC") + + datetime.timedelta(days=-5), + ) ) ).scalar() eq_(r, exp) diff --git a/test/orm/test_bundle.py b/test/orm/test_bundle.py index 9d1d0b61b7..49feb32f0b 100644 --- a/test/orm/test_bundle.py +++ b/test/orm/test_bundle.py @@ -1,15 +1,18 @@ +from sqlalchemy import exc from sqlalchemy import ForeignKey from sqlalchemy import func from sqlalchemy import Integer from sqlalchemy import select from sqlalchemy import String from sqlalchemy import testing +from sqlalchemy import tuple_ from sqlalchemy.orm import aliased from sqlalchemy.orm import Bundle from sqlalchemy.orm import mapper from sqlalchemy.orm import relationship from sqlalchemy.orm import Session from sqlalchemy.sql.elements import ClauseList +from sqlalchemy.testing import assert_raises_message from sqlalchemy.testing import AssertsCompiledSQL from sqlalchemy.testing import eq_ from sqlalchemy.testing import fixtures @@ -83,6 +86,34 @@ class BundleTest(fixtures.MappedTest, AssertsCompiledSQL): ) sess.commit() + def test_tuple_suggests_bundle(self, connection): + Data, Other = self.classes("Data", "Other") + + sess = Session(connection) + q = sess.query(tuple_(Data.id, Other.id)).join(Data.others) + + assert_raises_message( + exc.CompileError, + r"Most backends don't support SELECTing from a tuple\(\) object. " + "If this is an ORM query, consider using the Bundle object.", + q.all, + ) + + def test_tuple_suggests_bundle_future(self, connection): + Data, Other = self.classes("Data", "Other") + + stmt = select(tuple_(Data.id, Other.id)).join(Data.others) + + sess = Session(connection, future=True) + + assert_raises_message( + exc.CompileError, + r"Most backends don't support SELECTing from a tuple\(\) object. " + "If this is an ORM query, consider using the Bundle object.", + sess.execute, + stmt, + ) + def test_same_named_col_clauselist(self): Data, Other = self.classes("Data", "Other") bundle = Bundle("pk", Data.id, Other.id) diff --git a/test/sql/test_operators.py b/test/sql/test_operators.py index 7a027e28a0..e5835a7498 100644 --- a/test/sql/test_operators.py +++ b/test/sql/test_operators.py @@ -2791,7 +2791,7 @@ class TupleTypingTest(fixtures.TestBase): ) t1 = tuple_(a, b, c) expr = t1 == (3, "hi", "there") - self._assert_types([bind.type for bind in expr.right.element.clauses]) + self._assert_types([bind.type for bind in expr.right.clauses]) def test_type_coercion_on_in(self): a, b, c = ( @@ -2803,7 +2803,8 @@ class TupleTypingTest(fixtures.TestBase): expr = t1.in_([(3, "hi", "there"), (4, "Q", "P")]) eq_(len(expr.right.value), 2) - self._assert_types(expr.right._expanding_in_types) + + self._assert_types(expr.right.type.types) class InSelectableTest(fixtures.TestBase, testing.AssertsCompiledSQL): diff --git a/test/sql/test_query.py b/test/sql/test_query.py index bca7c262b7..3662a6e72d 100644 --- a/test/sql/test_query.py +++ b/test/sql/test_query.py @@ -179,6 +179,18 @@ class QueryTest(fixtures.TestBase): assert row.x == True # noqa assert row.y == False # noqa + def test_select_tuple(self, connection): + connection.execute( + users.insert(), {"user_id": 1, "user_name": "apples"}, + ) + + assert_raises_message( + exc.CompileError, + r"Most backends don't support SELECTing from a tuple\(\) object.", + connection.execute, + select(tuple_(users.c.user_id, users.c.user_name)), + ) + def test_like_ops(self, connection): connection.execute( users.insert(), diff --git a/test/sql/test_select.py b/test/sql/test_select.py index be39fe46bd..4c00cb53c7 100644 --- a/test/sql/test_select.py +++ b/test/sql/test_select.py @@ -6,6 +6,7 @@ from sqlalchemy import MetaData from sqlalchemy import select from sqlalchemy import String from sqlalchemy import Table +from sqlalchemy import tuple_ from sqlalchemy.sql import column from sqlalchemy.sql import table from sqlalchemy.testing import assert_raises_message @@ -197,3 +198,27 @@ class FutureSelectTest(fixtures.TestBase, AssertsCompiledSQL): select(table1).filter_by, foo="bar", ) + + def test_select_tuple_outer(self): + stmt = select(tuple_(table1.c.myid, table1.c.name)) + + assert_raises_message( + exc.CompileError, + r"Most backends don't support SELECTing from a tuple\(\) object. " + "If this is an ORM query, consider using the Bundle object.", + stmt.compile, + ) + + def test_select_tuple_subquery(self): + subq = select( + table1.c.name, tuple_(table1.c.myid, table1.c.name) + ).subquery() + + stmt = select(subq.c.name) + + # if we aren't fetching it, then render it + self.assert_compile( + stmt, + "SELECT anon_1.name FROM (SELECT mytable.name AS name, " + "(mytable.myid, mytable.name) AS anon_2 FROM mytable) AS anon_1", + )