From: Mike Bayer Date: Tue, 6 Jul 2021 15:26:53 +0000 (-0400) Subject: labeling refactor X-Git-Tag: rel_1_4_21~7^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=707e5d70fcdcfaaddcd0aaee51f4f1b881e5e3e2;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git labeling refactor To service #6718 and #6710, the system by which columns are given labels in a SELECT statement as well as the system that gives them keys in a .c or .selected_columns collection have been refactored to provide a single source of truth for both, in constrast to the previous approach that included similar logic repeated in slightly different ways. Main ideas: 1. ColumnElement attributes ._label, ._anon_label, ._key_label are renamed to include the letters "tq", meaning "table-qualified" - these labels are only used when rendering a SELECT that has LABEL_STYLE_TABLENAME_PLUS_COL for its label style; as this label style is primarily legacy, the "tq" names should be isolated so that in a 2.0 style application these aren't being used at all 2. The means by which the "labels" and "proxy keys" for the elements of a SELECT has been centralized to a single source of truth; previously, the three of _generate_columns_plus_names, _generate_fromclause_column_proxies, and _column_naming_convention all had duplicated rules between them, as well as that there were a little bit of labeling rules in compiler._label_select_column as well; by this we mean that the various "anon_label" "anon_key" methods on ColumnElement were called by all four of these methods, where there were many cases where it was necessary that one method comes up with the same answer as another of the methods. This has all been centralized into _generate_columns_plus_names for all the names except the "proxy key", which is generated by _column_naming_convention. 3. compiler._label_select_column has been rewritten to both not make any naming decisions nor any "proxy key" decisions, only whether to label or not to label; the _generate_columns_plus_names method gives it the information, where the proxy keys come from _column_naming_convention; previously, these proxy keys were matched based on restatement of similar (but not really the same) logic in two places. The heuristics of "whether to label or not to label" are also reorganized to be much easier to read and understand. 4. a new method compiler._label_returning_column is added for dialects to use in their "generate returning columns" methods. A github search reveals a small number of third party dialects also doing this using the prior _label_select_column method so we try to make sure _label_select_column continues to work the exact same way for that specific use case; for the "SELECT" use case it now needs 5. After some attempts to do it different ways, for the case where _proxy_key is giving us some kind of anon label, we are hard changing it to "_no_label" right now, as there's not currently a way to fully match anonymized labels from stmt.c or stmt.selected_columns to what will be in the result map. The idea of "_no_label" is to encourage the user to use label('name') for columns they want to be able to target by string name that don't have a natural name. Change-Id: I7a92a66f3a7e459ccf32587ac0a3c306650daf11 --- diff --git a/lib/sqlalchemy/dialects/firebird/base.py b/lib/sqlalchemy/dialects/firebird/base.py index 61e3e45080..91e2c04a7e 100644 --- a/lib/sqlalchemy/dialects/firebird/base.py +++ b/lib/sqlalchemy/dialects/firebird/base.py @@ -539,7 +539,7 @@ class FBCompiler(sql.compiler.SQLCompiler): def returning_clause(self, stmt, returning_cols): columns = [ - self._label_select_column(None, c, True, False, {}) + self._label_returning_column(stmt, c) for c in expression._select_iterables(returning_cols) ] diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py index 67d31226c3..c11166735c 100644 --- a/lib/sqlalchemy/dialects/mssql/base.py +++ b/lib/sqlalchemy/dialects/mssql/base.py @@ -2010,11 +2010,9 @@ class MSSQLCompiler(compiler.SQLCompiler): # necessarily used an expensive KeyError in order to match. columns = [ - self._label_select_column( - None, + self._label_returning_column( + stmt, adapter.traverse(c), - True, - False, {"result_map_targets": (c,)}, ) for c in expression._select_iterables(returning_cols) diff --git a/lib/sqlalchemy/dialects/postgresql/base.py b/lib/sqlalchemy/dialects/postgresql/base.py index ea2eda902f..6a8ee565d5 100644 --- a/lib/sqlalchemy/dialects/postgresql/base.py +++ b/lib/sqlalchemy/dialects/postgresql/base.py @@ -2304,7 +2304,7 @@ class PGCompiler(compiler.SQLCompiler): def returning_clause(self, stmt, returning_cols): columns = [ - self._label_select_column(None, c, True, False, {}) + self._label_returning_column(stmt, c) for c in expression._select_iterables(returning_cols) ] diff --git a/lib/sqlalchemy/engine/cursor.py b/lib/sqlalchemy/engine/cursor.py index 09c6a4db77..5e6078f866 100644 --- a/lib/sqlalchemy/engine/cursor.py +++ b/lib/sqlalchemy/engine/cursor.py @@ -728,12 +728,18 @@ class LegacyCursorResultMetaData(CursorResultMetaData): result = map_.get(key if self.case_sensitive else key.lower()) elif isinstance(key, expression.ColumnElement): if ( - key._label - and (key._label if self.case_sensitive else key._label.lower()) + key._tq_label + and ( + key._tq_label + if self.case_sensitive + else key._tq_label.lower() + ) in map_ ): result = map_[ - key._label if self.case_sensitive else key._label.lower() + key._tq_label + if self.case_sensitive + else key._tq_label.lower() ] elif ( hasattr(key, "name") diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index d691b8e1d8..530c0a112b 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -1729,7 +1729,7 @@ class Mapper( if isinstance(col, expression.Label): # new in 1.4, get column property against expressions # to be addressable in subqueries - col.key = col._key_label = key + col.key = col._tq_key_label = key self.columns.add(col, key) for col in prop.columns + prop._orig_columns: diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index 4b3b2c293c..aa71c0cb4e 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -35,7 +35,6 @@ from . import crud from . import elements from . import functions from . import operators -from . import roles from . import schema from . import selectable from . import sqltypes @@ -612,7 +611,7 @@ class SQLCompiler(Compiled): _loose_column_name_matching = False """tell the result object that the SQL staement is textual, wants to match - up to Column objects, and may be using the ._label in the SELECT rather + up to Column objects, and may be using the ._tq_label in the SELECT rather than the base name. """ @@ -1457,8 +1456,8 @@ class SQLCompiler(Compiled): if add_to_result_map is not None: targets = (column, name, column.key) + result_map_targets - if column._label: - targets += (column._label,) + if column._tq_label: + targets += (column._tq_label,) add_to_result_map(name, orig_name, targets, column.type) @@ -2816,6 +2815,24 @@ class SQLCompiler(Compiled): ) self._result_columns.append((keyname, name, objects, type_)) + def _label_returning_column(self, stmt, column, column_clause_args=None): + """Render a column with necessary labels inside of a RETURNING clause. + + This method is provided for individual dialects in place of calling + the _label_select_column method directly, so that the two use cases + of RETURNING vs. SELECT can be disambiguated going forward. + + .. versionadded:: 1.4.21 + + """ + return self._label_select_column( + None, + column, + True, + False, + {} if column_clause_args is None else column_clause_args, + ) + def _label_select_column( self, select, @@ -2824,6 +2841,8 @@ class SQLCompiler(Compiled): asfrom, column_clause_args, name=None, + proxy_name=None, + fallback_label_name=None, within_columns_clause=True, column_is_repeated=False, need_column_expressions=False, @@ -2867,9 +2886,17 @@ class SQLCompiler(Compiled): else: add_to_result_map = None - if not within_columns_clause: - result_expr = col_expr - elif isinstance(column, elements.Label): + # this method is used by some of the dialects for RETURNING, + # which has different inputs. _label_returning_column was added + # as the better target for this now however for 1.4 we will keep + # _label_select_column directly compatible with this use case. + # these assertions right now set up the current expected inputs + assert within_columns_clause, ( + "_label_select_column is only relevant within " + "the columns clause of a SELECT or RETURNING" + ) + + if isinstance(column, elements.Label): if col_expr is not column: result_expr = _CompileLabel( col_expr, column.name, alt_names=(column.element,) @@ -2877,50 +2904,91 @@ class SQLCompiler(Compiled): else: result_expr = col_expr - elif select is not None and name: - result_expr = _CompileLabel( - col_expr, name, alt_names=(column._key_label,) - ) - elif ( - asfrom - and isinstance(column, elements.ColumnClause) - and not column.is_literal - and column.table is not None - and not isinstance(column.table, selectable.Select) - ): - result_expr = _CompileLabel( - col_expr, - coercions.expect(roles.TruncatedLabelRole, column.name), - alt_names=(column.key,), - ) - elif ( - not isinstance(column, elements.TextClause) - and ( - not isinstance(column, elements.UnaryExpression) - or column.wraps_column_expression - or asfrom - ) - and ( - not hasattr(column, "name") - or isinstance(column, functions.FunctionElement) - ) - ): - result_expr = _CompileLabel( - col_expr, - column._anon_name_label - if not column_is_repeated - else column._dedupe_label_anon_label, - ) - elif col_expr is not column: - # TODO: are we sure "column" has a .name and .key here ? - # assert isinstance(column, elements.ColumnClause) + elif name: + # here, _columns_plus_names has determined there's an explicit + # label name we need to use. this is the default for + # tablenames_plus_columnnames as well as when columns are being + # deduplicated on name + + assert ( + proxy_name is not None + ), "proxy_name is required if 'name' is passed" + result_expr = _CompileLabel( col_expr, - coercions.expect(roles.TruncatedLabelRole, column.name), - alt_names=(column.key,), + name, + alt_names=( + proxy_name, + # this is a hack to allow legacy result column lookups + # to work as they did before; this goes away in 2.0. + # TODO: this only seems to be tested indirectly + # via test/orm/test_deprecations.py. should be a + # resultset test for this + column._tq_label, + ), ) else: - result_expr = col_expr + # determine here whether this column should be rendered in + # a labelled context or not, as we were given no required label + # name from the caller. Here we apply heuristics based on the kind + # of SQL expression involved. + + if col_expr is not column: + # type-specific expression wrapping the given column, + # so we render a label + render_with_label = True + elif isinstance(column, elements.ColumnClause): + # table-bound column, we render its name as a label if we are + # inside of a subquery only + render_with_label = ( + asfrom + and not column.is_literal + and column.table is not None + ) + elif isinstance(column, elements.TextClause): + render_with_label = False + elif isinstance(column, elements.UnaryExpression): + render_with_label = column.wraps_column_expression or asfrom + elif ( + # general class of expressions that don't have a SQL-column + # addressible name. includes scalar selects, bind parameters, + # SQL functions, others + not isinstance(column, elements.NamedColumn) + # deeper check that indicates there's no natural "name" to + # this element, which accommodates for custom SQL constructs + # that might have a ".name" attribute (but aren't SQL + # functions) but are not implementing this more recently added + # base class. in theory the "NamedColumn" check should be + # enough, however here we seek to maintain legacy behaviors + # as well. + and column._non_anon_label is None + ): + render_with_label = True + else: + render_with_label = False + + if render_with_label: + if not fallback_label_name: + # used by the RETURNING case right now. we generate it + # here as 3rd party dialects may be referring to + # _label_select_column method directly instead of the + # just-added _label_returning_column method + assert not column_is_repeated + fallback_label_name = column._anon_name_label + + fallback_label_name = ( + elements._truncated_label(fallback_label_name) + if not isinstance( + fallback_label_name, elements._truncated_label + ) + else fallback_label_name + ) + + result_expr = _CompileLabel( + col_expr, fallback_label_name, alt_names=(proxy_name,) + ) + else: + result_expr = col_expr column_clause_args.update( within_columns_clause=within_columns_clause, @@ -3092,10 +3160,18 @@ class SQLCompiler(Compiled): asfrom, column_clause_args, name=name, + proxy_name=proxy_name, + fallback_label_name=fallback_label_name, column_is_repeated=repeated, need_column_expressions=need_column_expressions, ) - for name, column, repeated in compile_state.columns_plus_names + for ( + name, + proxy_name, + fallback_label_name, + column, + repeated, + ) in compile_state.columns_plus_names ] if c is not None ] @@ -3110,6 +3186,8 @@ class SQLCompiler(Compiled): name for ( key, + proxy_name, + fallback_label_name, name, repeated, ) in compile_state.columns_plus_names @@ -3118,6 +3196,8 @@ class SQLCompiler(Compiled): name for ( key, + proxy_name, + fallback_label_name, name, repeated, ) in compile_state_wraps_for.columns_plus_names diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py index 173314abe7..f95fa143e9 100644 --- a/lib/sqlalchemy/sql/elements.py +++ b/lib/sqlalchemy/sql/elements.py @@ -687,19 +687,21 @@ class ColumnElement( foreign_keys = [] _proxies = () - _label = None + _tq_label = None """The named label that can be used to target - this column in a result set. + this column in a result set in a "table qualified" context. This label is almost always the label used when - rendering AS