From: Mike Bayer Date: Mon, 16 Dec 2019 21:38:06 +0000 (-0500) Subject: Do the CompoundSelect check for number of columns in the compile phase X-Git-Tag: rel_1_4_0b1~596 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e7c78be2e737591d637b6acde6117893fd29dfe0;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Do the CompoundSelect check for number of columns in the compile phase Starting to go forward with the general idea of moving more of Core / ORM construction into the compile phase. Bigger initiatives like the refactor of Query will follow onto this. Change-Id: I0f364d3182e21e32ed85ef34cfd11fd9d11cf653 --- diff --git a/doc/build/changelog/migration_14.rst b/doc/build/changelog/migration_14.rst index f2dd2745ea..6c91c40f4f 100644 --- a/doc/build/changelog/migration_14.rst +++ b/doc/build/changelog/migration_14.rst @@ -18,6 +18,37 @@ What's New in SQLAlchemy 1.4? implicit behaviors and rarely-used API flags that complicate the internals and hinder performance will be removed. +Behavioral Changes - General +============================ + +.. _change_change_deferred_construction: + + +Many Core and ORM statement objects now perform much of their validation in the compile phase +--------------------------------------------------------------------------------------------- + +A major initiative in the 1.4 series is to approach the model of both Core SQL +statements as well as the ORM Query to allow for an efficient, cacheable model +of statement creation and compilation, where the compilation step would be +cached, based on a cache key generated by the created statement object, which +itself is newly created for each use. Towards this goal, much of the Python +computation which occurs within the construction of statements, particularly +the ORM :class:`.Query`, is being moved to occur only when the statement is +invoked. This means that some of the error messages which can arise based on +arguments passed to the object will no longer be raised immediately, and +instead will occur only when the statement is invoked. + +Error conditions which fall under this category include: + +* when a :class:`.CompoundSelect` is constructed (e.g. a UNION, EXCEPT, etc.) + and the SELECT statements passed do not have the same number of columns, a + :class:`.CompileError` is now raised to this effect; previously, a + :class:`.ArgumentError` would be raised immediately upon statement + construction. + +* To be continued... + + API Changes - Core ================== diff --git a/doc/build/changelog/unreleased_14/checks_deferred_to_compile.rst b/doc/build/changelog/unreleased_14/checks_deferred_to_compile.rst new file mode 100644 index 0000000000..bbaeac2150 --- /dev/null +++ b/doc/build/changelog/unreleased_14/checks_deferred_to_compile.rst @@ -0,0 +1,19 @@ +.. change:: + :tags: change, orm, sql + + A selection of Core and ORM query objects now perform much more of their + Python computational tasks within the compile step, rather than at + construction time. This is to support an upcoming caching model that will + provide for caching of the compiled statement structure based on a cache + key that is derived from the statement construct, which itself is expected + to be newly constructed in Python code each time it is used. This means + that the internal state of these objects may not be the same as it used to + be, as well as that some but not all error raise scenarios for various + kinds of argument validation will occur within the compilation / execution + phase, rather than at statement construction time. See the migration + notes linked below for complete details. + + .. seealso:: + + :ref:`change_deferred_construction` + diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index a28ce465a9..4ec3b93ea7 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -2149,7 +2149,9 @@ class SQLCompiler(Compiled): if not populate_result_map and "add_to_result_map" in kwargs: del kwargs["add_to_result_map"] - froms = self._setup_select_stack(select, entry, asfrom, lateral) + froms = self._setup_select_stack( + select, entry, asfrom, lateral, compound_index + ) column_clause_args = kwargs.copy() column_clause_args.update( @@ -2254,10 +2256,33 @@ class SQLCompiler(Compiled): hint_text = self.get_select_hint_text(byfrom) return hint_text, byfrom - def _setup_select_stack(self, select, entry, asfrom, lateral): + def _setup_select_stack( + self, select, entry, asfrom, lateral, compound_index + ): correlate_froms = entry["correlate_froms"] asfrom_froms = entry["asfrom_froms"] + if compound_index > 0: + # note this is cached + select_0 = entry["selectable"].selects[0] + if select_0._is_select_container: + select_0 = select_0.element + numcols = len(select_0.selected_columns) + # numcols = len(select_0._columns_plus_names) + if len(select._columns_plus_names) != numcols: + raise exc.CompileError( + "All selectables passed to " + "CompoundSelect must have identical numbers of " + "columns; select #%d has %d columns, select " + "#%d has %d" + % ( + 1, + numcols, + compound_index + 1, + len(select.selected_columns), + ) + ) + if asfrom and not lateral: froms = select._get_display_froms( explicit_correlate_froms=correlate_froms.difference( diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py index 5f609f8fdc..bece0b3c58 100644 --- a/lib/sqlalchemy/sql/selectable.py +++ b/lib/sqlalchemy/sql/selectable.py @@ -2782,31 +2782,12 @@ class CompoundSelect(GenerativeSelect): def __init__(self, keyword, *selects, **kwargs): self._auto_correlate = kwargs.pop("correlate", False) self.keyword = keyword - self.selects = [] - - numcols = None - - # some DBs do not like ORDER BY in the inner queries of a UNION, etc. - for n, s in enumerate(selects): - s = coercions.expect(roles.CompoundElementRole, s) - - if not numcols: - numcols = len(s.selected_columns) - elif len(s.selected_columns) != numcols: - raise exc.ArgumentError( - "All selectables passed to " - "CompoundSelect must have identical numbers of " - "columns; select #%d has %d columns, select " - "#%d has %d" - % ( - 1, - len(self.selects[0].selected_columns), - n + 1, - len(s.selected_columns), - ) - ) - - self.selects.append(s.self_group(against=self)) + self.selects = [ + coercions.expect(roles.CompoundElementRole, s).self_group( + against=self + ) + for s in selects + ] GenerativeSelect.__init__(self, **kwargs) diff --git a/test/sql/test_compiler.py b/test/sql/test_compiler.py index 486b546828..54c3e2f6ed 100644 --- a/test/sql/test_compiler.py +++ b/test/sql/test_compiler.py @@ -1995,13 +1995,11 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL): def test_compound_selects(self): assert_raises_message( - exc.ArgumentError, + exc.CompileError, "All selectables passed to CompoundSelect " "must have identical numbers of columns; " "select #1 has 2 columns, select #2 has 3", - union, - table3.select(), - table1.select(), + union(table3.select(), table1.select()).compile, ) x = union(