Skip to content

Commit

Permalink
Catch aggregate calls earlier in indexes.
Browse files Browse the repository at this point in the history
  • Loading branch information
dnwpark committed May 16, 2024
1 parent 8a0c162 commit 5fa6dc7
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 26 deletions.
9 changes: 0 additions & 9 deletions edb/ir/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -490,15 +490,6 @@ def find_potentially_visible(
return visible_paths


def contains_set_of_op(ir: irast.Base) -> bool:
def flt(n: irast.Call) -> bool:
return any(
arg.param_typemod == ft.TypeModifier.SetOfType
for arg in n.args.values()
)
return bool(ast.find_children(ir, irast.Call, flt, terminate_early=True))


def is_singleton_set_of_call(
call: irast.Call
) -> bool:
Expand Down
20 changes: 15 additions & 5 deletions edb/schema/indexes.py
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,7 @@ def compile_expr_field(
value: s_expr.Expression,
track_schema_ref_exprs: bool=False,
) -> s_expr.CompiledExpression:
from edb.ir import ast as irast
from edb.ir import utils as irutils

if field.name in {'expr', 'except_expr'}:
Expand Down Expand Up @@ -850,11 +851,20 @@ def compile_expr_field(
has_multi = True
break

if has_multi and irutils.contains_set_of_op(expr.irast):
raise errors.SchemaDefinitionError(
"cannot use aggregate functions or operators "
"in an index expression",
span=self.span,
if set_of_op := irutils.find_set_of_op(
expr.irast,
has_multi,
):
label = (
'function'
if isinstance(set_of_op, irast.FunctionCall) else
'operator'
)
op_name = str(set_of_op.func_shortname)
raise errors.UnsupportedFeatureError(
f"cannot use aggregate {label} '{op_name}' "
f"in an index expression",
span=set_of_op.span
)

# compile the expression to sql to preempt errors downstream
Expand Down
4 changes: 2 additions & 2 deletions tests/test_constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -1400,7 +1400,7 @@ async def test_constraints_ddl_08(self):
""")

async with self.assertRaisesRegexTx(
edgedb.UnsupportedFeatureError,
edgedb.InvalidConstraintDefinitionError,
"cannot use aggregate operator 'std::EXISTS' "
"in a non-aggregating constraint",
):
Expand All @@ -1411,7 +1411,7 @@ async def test_constraints_ddl_08(self):
""")

async with self.assertRaisesRegexTx(
edgedb.UnsupportedFeatureError,
edgedb.InvalidConstraintDefinitionError,
"cannot use aggregate operator 'std::EXISTS' "
"in a non-aggregating constraint",
):
Expand Down
16 changes: 14 additions & 2 deletions tests/test_edgeql_ddl.py
Original file line number Diff line number Diff line change
Expand Up @@ -13218,15 +13218,27 @@ async def test_edgeql_ddl_index_07(self):
async def test_edgeql_ddl_index_08(self):
async with self.assertRaisesRegexTx(
edgedb.UnsupportedFeatureError,
'set returning operator std::DISTINCT is not supported '
'in singleton expressions'):
"cannot use aggregate operator 'std::DISTINCT' "
"in an index expression"):
await self.con.execute(r"""
CREATE TYPE default::IndexNonSingletonTest {
CREATE PROPERTY has_bad_index: std::str;
CREATE INDEX ON (DISTINCT (.has_bad_index));
};
""")

async def test_edgeql_ddl_index_09(self):
async with self.assertRaisesRegexTx(
edgedb.UnsupportedFeatureError,
"cannot use aggregate function 'std::count' "
"in an index expression"):
await self.con.execute(r"""
CREATE TYPE default::IndexNonSingletonTest {
CREATE PROPERTY has_bad_index: std::str;
CREATE INDEX ON (std::count (.has_bad_index));
};
""")

async def test_edgeql_ddl_abstract_index_01(self):
for _ in range(2):
await self.con.execute('''
Expand Down
12 changes: 6 additions & 6 deletions tests/test_indexes.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,9 @@ async def test_index_05(self):
)

async with self.assertRaisesRegexTx(
edgedb.SchemaDefinitionError,
"cannot use aggregate functions or operators in an "
"index expression",
edgedb.UnsupportedFeatureError,
"cannot use aggregate operator 'std::EXISTS' "
"in an index expression",
):
await self.con.execute(
"""
Expand All @@ -185,9 +185,9 @@ async def test_index_05(self):
)

async with self.assertRaisesRegexTx(
edgedb.SchemaDefinitionError,
"cannot use aggregate functions or operators in an "
"index expression",
edgedb.UnsupportedFeatureError,
"cannot use aggregate function 'std::count' "
"in an index expression",
):
await self.con.execute(
"""
Expand Down
18 changes: 16 additions & 2 deletions tests/test_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -2875,8 +2875,8 @@ def test_schema_constraint_singleton_05b(self):

@tb.must_fail(
errors.UnsupportedFeatureError,
'set returning operator std::DISTINCT is not supported '
'in singleton expressions',
"cannot use aggregate operator 'std::DISTINCT' "
"in an index expression",
)
def test_schema_index_non_singleton_01(self):
"""
Expand All @@ -2887,6 +2887,20 @@ def test_schema_index_non_singleton_01(self):
}
"""

@tb.must_fail(
errors.UnsupportedFeatureError,
"cannot use aggregate function 'std::count' "
"in an index expression",
)
def test_schema_index_non_singleton_02(self):
"""
type IndexNonSingletonTest {
property has_bad_index -> str;
index on (count(.has_bad_index))
}
"""

@tb.must_fail(
errors.SchemaError,
"cannot create union \\(test::X | test::Y\\) with property 'a' using "
Expand Down

0 comments on commit 5fa6dc7

Please sign in to comment.