Skip to content

Commit

Permalink
revise approach for bundle deduping
Browse files Browse the repository at this point in the history
Revise the approach from 7d6d7ef to make a special case for
Bundle-targeted columns entirely, and don't involve the
_label_convention() callable.   Add tests for select() with
tablename labeling convention.

Fixes: #11347
Change-Id: I1d15523de5709d45b2b69bc17724831ac3425791
(cherry picked from commit 83f8dd5)
  • Loading branch information
zzzeek committed May 4, 2024
1 parent 4e1c092 commit cbc2b9c
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 20 deletions.
18 changes: 11 additions & 7 deletions lib/sqlalchemy/orm/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ def _column_naming_convention(
) -> _LabelConventionCallable:
if legacy:

def name(col, col_name=None, cancel_dedupe=False):
def name(col, col_name=None):
if col_name:
return col_name
else:
Expand Down Expand Up @@ -3050,7 +3050,10 @@ def __init__(
if not is_current_entities or column._is_text_clause:
self._label_name = None
else:
self._label_name = compile_state._label_convention(column)
if parent_bundle:
self._label_name = column._proxy_key
else:
self._label_name = compile_state._label_convention(column)

if parent_bundle:
parent_bundle._entities.append(self)
Expand Down Expand Up @@ -3144,11 +3147,12 @@ def __init__(
self.raw_column_index = raw_column_index

if is_current_entities:
self._label_name = compile_state._label_convention(
column,
col_name=orm_key,
cancel_dedupe=parent_bundle is not None,
)
if parent_bundle:
self._label_name = orm_key if orm_key else column._proxy_key
else:
self._label_name = compile_state._label_convention(
column, col_name=orm_key
)
else:
self._label_name = None

Expand Down
3 changes: 1 addition & 2 deletions lib/sqlalchemy/sql/selectable.py
Original file line number Diff line number Diff line change
Expand Up @@ -4556,14 +4556,13 @@ def _column_naming_convention(
def go(
c: Union[ColumnElement[Any], TextClause],
col_name: Optional[str] = None,
cancel_dedupe: bool = False,
) -> Optional[str]:
if is_text_clause(c):
return None
elif TYPE_CHECKING:
assert is_column_element(c)

if not dedupe or cancel_dedupe:
if not dedupe:
name = c._proxy_key
if name is None:
name = "_no_label"
Expand Down
109 changes: 98 additions & 11 deletions test/orm/test_bundle.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from sqlalchemy import func
from sqlalchemy import Integer
from sqlalchemy import select
from sqlalchemy import SelectLabelStyle
from sqlalchemy import String
from sqlalchemy import testing
from sqlalchemy import tuple_
Expand Down Expand Up @@ -159,29 +160,68 @@ def test_c_attr(self):
select(b1.c.d1, b1.c.d2), "SELECT data.d1, data.d2 FROM data"
)

@testing.variation("stmt_type", ["legacy", "newstyle"])
def test_dupe_col_name(self, stmt_type):
@testing.variation(
"stmt_type", ["legacy", "newstyle", "newstyle_w_label_conv"]
)
@testing.variation("col_type", ["orm", "core"])
def test_dupe_col_name(self, stmt_type, col_type):
"""test #11347"""
Data = self.classes.Data
sess = fixture_session()

b1 = Bundle("b1", Data.d1, Data.d3)
if col_type.orm:
b1 = Bundle("b1", Data.d1, Data.d3)
cols = Data.d1, Data.d2
elif col_type.core:
data_table = self.tables.data
b1 = Bundle("b1", data_table.c.d1, data_table.c.d3)
cols = data_table.c.d1, data_table.c.d2
else:
col_type.fail()

if stmt_type.legacy:
row = (
sess.query(Data.d1, Data.d2, b1)
sess.query(cols[0], cols[1], b1)
.filter(Data.d1 == "d0d1")
.one()
)
elif stmt_type.newstyle:
row = sess.execute(
select(Data.d1, Data.d2, b1).filter(Data.d1 == "d0d1")
select(cols[0], cols[1], b1).filter(Data.d1 == "d0d1")
).one()
elif stmt_type.newstyle_w_label_conv:
row = sess.execute(
select(cols[0], cols[1], b1)
.filter(Data.d1 == "d0d1")
.set_label_style(
SelectLabelStyle.LABEL_STYLE_TABLENAME_PLUS_COL
)
).one()
else:
stmt_type.fail()

if stmt_type.newstyle_w_label_conv:
# decision is made here that even if a SELECT with the
# "tablename_plus_colname" label style, within a Bundle we still
# use straight column name, even though the overall row
# uses tablename_colname
eq_(
row._mapping,
{"data_d1": "d0d1", "data_d2": "d0d2", "b1": ("d0d1", "d0d3")},
)
else:
eq_(
row._mapping,
{"d1": "d0d1", "d2": "d0d2", "b1": ("d0d1", "d0d3")},
)

eq_(row[2]._mapping, {"d1": "d0d1", "d3": "d0d3"})

@testing.variation("stmt_type", ["legacy", "newstyle"])
def test_dupe_col_name_nested(self, stmt_type):
@testing.variation(
"stmt_type", ["legacy", "newstyle", "newstyle_w_label_conv"]
)
@testing.variation("col_type", ["orm", "core"])
def test_dupe_col_name_nested(self, stmt_type, col_type):
"""test #11347"""
Data = self.classes.Data
sess = fixture_session()
Expand All @@ -193,9 +233,18 @@ def proc(row):

return proc

b1 = DictBundle("b1", Data.d1, Data.d3)
b2 = DictBundle("b2", Data.d2, Data.d3)
b3 = DictBundle("b3", Data.d2, Data.d3, b1, b2)
if col_type.core:
data_table = self.tables.data

b1 = DictBundle("b1", data_table.c.d1, data_table.c.d3)
b2 = DictBundle("b2", data_table.c.d2, data_table.c.d3)
b3 = DictBundle("b3", data_table.c.d2, data_table.c.d3, b1, b2)
elif col_type.orm:
b1 = DictBundle("b1", Data.d1, Data.d3)
b2 = DictBundle("b2", Data.d2, Data.d3)
b3 = DictBundle("b3", Data.d2, Data.d3, b1, b2)
else:
col_type.fail()

if stmt_type.legacy:
row = (
Expand All @@ -207,7 +256,45 @@ def proc(row):
row = sess.execute(
select(Data.d1, Data.d2, b3).filter(Data.d1 == "d0d1")
).one()

elif stmt_type.newstyle_w_label_conv:
row = sess.execute(
select(Data.d1, Data.d2, b3)
.filter(Data.d1 == "d0d1")
.set_label_style(
SelectLabelStyle.LABEL_STYLE_TABLENAME_PLUS_COL
)
).one()
else:
stmt_type.fail()

if stmt_type.newstyle_w_label_conv:
eq_(
row._mapping,
{
"data_d1": "d0d1",
"data_d2": "d0d2",
"b3": {
"d2": "d0d2",
"d3": "d0d3",
"b1": {"d1": "d0d1", "d3": "d0d3"},
"b2": {"d2": "d0d2", "d3": "d0d3"},
},
},
)
else:
eq_(
row._mapping,
{
"d1": "d0d1",
"d2": "d0d2",
"b3": {
"d2": "d0d2",
"d3": "d0d3",
"b1": {"d1": "d0d1", "d3": "d0d3"},
"b2": {"d2": "d0d2", "d3": "d0d3"},
},
},
)
eq_(
row[2],
{
Expand Down

0 comments on commit cbc2b9c

Please sign in to comment.