Skip to content

Commit 7ea372a

Browse files
fix(query): count and restrict expressions with an empty primary key
An expression with an empty primary key, such as `dj.U() & T` or `dj.U().aggr(T, n="count(*)")`, holds at most one row. Two operations on such expressions failed: - `len()` took the single-column branch and generated `count(DISTINCT )`, which is invalid SQL. It now returns 0 or 1 by checking whether the row exists. - Restricting a `dj.U()` aggregation hit a bare assert in `Aggregation.make_sql`. Removing the assert alone would not have been enough: the MySQL branch emitted HAVING only together with GROUP BY, so the restriction would have been dropped without notice. MySQL now emits HAVING without GROUP BY, and PostgreSQL takes the subquery path whenever the aggregation is restricted, with or without grouping attributes. Restriction conditions on an aggregation are now joined with str(), as the base where_clause already does, so restricting any aggregation by False no longer raises TypeError. Closes #1581 Closes #1580
1 parent a00b89f commit 7ea372a

2 files changed

Lines changed: 136 additions & 17 deletions

File tree

‎src/datajoint/expression.py‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,6 +1156,10 @@ def __len__(self):
11561156
Number of elements in the result set.
11571157
"""
11581158
result = self.make_subquery() if self._top else copy.copy(self)
1159+
if not result.primary_key:
1160+
# An empty primary key (e.g. ``dj.U() & table``) admits at most one row:
1161+
# count it by checking whether it exists.
1162+
return int(bool(result))
11591163
has_left_join = any(is_left for is_left, _ in result._joins)
11601164

11611165
# Build COUNT query - PostgreSQL requires different syntax for multi-column DISTINCT
@@ -1355,41 +1359,38 @@ def where_clause(self):
13551359
def make_sql(self, fields=None):
13561360
adapter = self.connection.adapter
13571361
fields = self.heading.as_sql(fields or self.heading.names, adapter=adapter)
1358-
assert self._grouping_attributes or not self.restriction
13591362
distinct = set(self.heading.names) == set(self.primary_key)
1363+
# Without grouping attributes (``dj.U().aggr(...)``), the whole input is one group.
1364+
group_by = (
1365+
" GROUP BY {}".format(", ".join(adapter.quote_identifier(col) for col in self._grouping_attributes))
1366+
if self._grouping_attributes
1367+
else ""
1368+
)
1369+
conditions = ")AND(".join(str(s) for s in self.restriction)
13601370

13611371
# PostgreSQL doesn't allow column aliases in HAVING clause (SQL standard).
13621372
# For PostgreSQL with restrictions, wrap aggregation in subquery and use WHERE.
1363-
use_subquery_for_having = adapter.backend == "postgresql" and self.restriction and self._grouping_attributes
1364-
1365-
if use_subquery_for_having:
1373+
if adapter.backend == "postgresql" and self.restriction:
13661374
# Generate inner query without HAVING
13671375
inner_sql = "SELECT {distinct}{fields} FROM {from_}{where}{group_by}".format(
13681376
distinct="DISTINCT " if distinct else "",
13691377
fields=fields,
13701378
from_=self.from_clause(),
13711379
where=self.where_clause(),
1372-
group_by=" GROUP BY {}".format(", ".join(adapter.quote_identifier(col) for col in self._grouping_attributes)),
1380+
group_by=group_by,
13731381
)
13741382
# Wrap in subquery with WHERE for the HAVING conditions
13751383
subquery_alias = adapter.quote_identifier(f"_aggr{next(self._subquery_alias_count)}")
1376-
outer_where = " WHERE (%s)" % ")AND(".join(self.restriction)
1377-
return f"SELECT * FROM ({inner_sql}) AS {subquery_alias}{outer_where}{self.sorting_clauses()}"
1384+
return f"SELECT * FROM ({inner_sql}) AS {subquery_alias} WHERE ({conditions}){self.sorting_clauses()}"
13781385
else:
1379-
# MySQL path: use HAVING directly
1380-
return "SELECT {distinct}{fields} FROM {from_}{where}{group_by}{sorting}".format(
1386+
# MySQL path: use HAVING directly, which MySQL also accepts without GROUP BY
1387+
return "SELECT {distinct}{fields} FROM {from_}{where}{group_by}{having}{sorting}".format(
13811388
distinct="DISTINCT " if distinct else "",
13821389
fields=fields,
13831390
from_=self.from_clause(),
13841391
where=self.where_clause(),
1385-
group_by=(
1386-
""
1387-
if not self.primary_key
1388-
else (
1389-
" GROUP BY {}".format(", ".join(adapter.quote_identifier(col) for col in self._grouping_attributes))
1390-
+ ("" if not self.restriction else " HAVING (%s)" % ")AND(".join(self.restriction))
1391-
)
1392-
),
1392+
group_by=group_by,
1393+
having=f" HAVING ({conditions})" if self.restriction else "",
13931394
sorting=self.sorting_clauses(),
13941395
)
13951396

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
"""
2+
Queries whose primary key is empty: ``dj.U() & table`` and ``dj.U().aggr(...)``.
3+
4+
An expression with an empty primary key holds at most one row. ``len()`` must
5+
count that row (#1581), and a restriction on a ``dj.U()`` aggregation must
6+
filter it (#1580). Both run on MySQL and PostgreSQL.
7+
"""
8+
9+
import time
10+
11+
import pytest
12+
13+
import datajoint as dj
14+
15+
16+
@pytest.fixture(scope="function")
17+
def schema_by_backend(connection_by_backend, db_creds_by_backend):
18+
"""Create a fresh schema per test, parameterized across backends."""
19+
backend = db_creds_by_backend["backend"]
20+
test_id = str(int(time.time() * 1000))[-8:]
21+
schema_name = f"djtest_emptypk_{backend}_{test_id}"[:64]
22+
23+
if connection_by_backend.is_connected:
24+
try:
25+
connection_by_backend.query(
26+
f"DROP DATABASE IF EXISTS {connection_by_backend.adapter.quote_identifier(schema_name)}"
27+
)
28+
except Exception:
29+
pass
30+
31+
schema = dj.Schema(schema_name, connection=connection_by_backend)
32+
yield schema
33+
34+
if connection_by_backend.is_connected:
35+
try:
36+
connection_by_backend.query(
37+
f"DROP DATABASE IF EXISTS {connection_by_backend.adapter.quote_identifier(schema_name)}"
38+
)
39+
except Exception:
40+
pass
41+
42+
43+
@pytest.fixture
44+
def File(schema_by_backend):
45+
@schema_by_backend
46+
class File(dj.Manual):
47+
definition = """
48+
file_id : int32
49+
---
50+
kind : varchar(16)
51+
"""
52+
53+
return File
54+
55+
56+
def test_len_of_universal_restriction(File):
57+
"""``dj.U() & table`` has one row when the table is non-empty, none otherwise."""
58+
assert len(dj.U() & File) == 0
59+
File.insert([(1, "data"), (2, "data"), (3, "meta")])
60+
assert len(dj.U() & File) == 1
61+
assert len(dj.U() & (File & "kind = 'meta'")) == 1
62+
assert len(dj.U() & (File & "kind = 'none'")) == 0
63+
64+
65+
def test_len_of_universal_aggregation_subquery(File):
66+
"""``len()`` of a subquery over a ``dj.U()`` aggregation, restricted or not."""
67+
File.insert([(1, "data"), (2, "data"), (3, "meta")])
68+
total = dj.U().aggr(File, n="count(*)").make_subquery()
69+
assert len(total) == 1
70+
assert len(total & "n = 3") == 1
71+
assert len(total & "n = 4") == 0
72+
73+
74+
def test_restrict_universal_aggregation(File):
75+
"""A restriction on the aggregated value of a ``dj.U()`` aggregation filters its row."""
76+
File.insert([(1, "data"), (2, "data"), (3, "meta")])
77+
total = dj.U().aggr(File, n="count(*)")
78+
79+
assert (total & "n = 3").to_dicts() == [{"n": 3}]
80+
assert (total & {"n": 3}).to_dicts() == [{"n": 3}]
81+
assert (total - "n = 4").to_dicts() == [{"n": 3}]
82+
assert (total & "n = 4").to_dicts() == []
83+
assert (total & {"n": 4}).to_dicts() == []
84+
assert (total - "n = 3").to_dicts() == []
85+
assert (total & False).to_dicts() == []
86+
87+
assert len(total & "n = 3") == 1
88+
assert len(total & "n = 4") == 0
89+
assert total & "n = 3"
90+
assert not total & "n = 4"
91+
92+
93+
def test_restrict_universal_aggregation_of_restricted_group(File):
94+
"""Conditions on the group (WHERE) and on the aggregate (HAVING) combine."""
95+
File.insert([(1, "data"), (2, "data"), (3, "meta")])
96+
data_count = dj.U().aggr(File & "kind = 'data'", n="count(*)")
97+
assert (data_count & "n = 2").to_dicts() == [{"n": 2}]
98+
assert (data_count & "n = 3").to_dicts() == []
99+
100+
101+
def test_restrict_by_restricted_universal_aggregation(File):
102+
"""A restricted ``dj.U()`` aggregation shares no attributes with ``File``:
103+
it keeps every row when non-empty and none when empty."""
104+
File.insert([(1, "data"), (2, "data"), (3, "meta")])
105+
total = dj.U().aggr(File, n="count(*)")
106+
assert len(File & (total & "n = 3")) == 3
107+
assert len(File & (total & "n = 4")) == 0
108+
assert len(File - (total & "n = 4")) == 3
109+
110+
111+
def test_restrict_grouped_aggregation(File):
112+
"""Restricting an aggregation with grouping attributes still filters by group."""
113+
File.insert([(1, "data"), (2, "data"), (3, "meta")])
114+
per_kind = dj.U("kind").aggr(File, n="count(*)")
115+
assert (per_kind & "n > 1").to_dicts() == [{"kind": "data", "n": 2}]
116+
assert len(per_kind & "n > 1") == 1
117+
assert (per_kind & {"kind": "meta"}).to_dicts() == [{"kind": "meta", "n": 1}]
118+
assert (per_kind & False).to_dicts() == []

0 commit comments

Comments
 (0)