Skip to content

Commit

Permalink
Fix ISEs in constant detection for fts::with_options (#7192)
Browse files Browse the repository at this point in the history
`fts::with_options` checks `irutils.is_const` to validate whether
language and weight_category are constant, but its notion of constant
(that it doesn't access the database) is not the one we need here.

Instead, skip doing a separate check, and just try to extract the
constant directly. We could do this with `as_const`, but I decided
to delete `as_const` and use `staeval` instead.

This makes it work with simple SELECT expressions also.
(Hitting this in an experimental branch is what caused me to run into
this.)

TODO: Probably we should also get rid of `is_const` too, and do
volatility checks in the places it is used.
  • Loading branch information
msullivan authored Apr 13, 2024
1 parent 93c2884 commit 7d8c4e4
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 25 deletions.
27 changes: 13 additions & 14 deletions edb/edgeql/compiler/func.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
from edb.common.typeutils import not_none

from edb.ir import ast as irast
from edb.ir import staeval
from edb.ir import utils as irutils

from edb.schema import constraints as s_constr
Expand Down Expand Up @@ -1043,35 +1044,33 @@ def compile_fts_with_options(
assert lang_ty

lang_domain = set() # languages that the fts index needs to support
if irutils.is_const(lang):
try:
lang_const = staeval.evaluate_to_python_val(lang, ctx.env.schema)
except staeval.UnsupportedExpressionError:
lang_const = None

if lang_const is not None:
# language is constant
# -> determine its only value at compile time
lang_const = irutils.as_const(lang)
assert lang_const
lang_domain.add(str(lang_const.value).lower())

lang_domain.add(lang_const.lower())
else:
# language is not constant
# -> use all possible values of the enum

enum_values = lang_ty.get_enum_values(ctx.env.schema)
assert enum_values
for enum_value in enum_values:
lang_domain.add(enum_value.lower())

# weight_category
weight_expr = call.args['weight_category'].expr
if not irutils.is_const(weight_expr):
try:
weight: str = staeval.evaluate_to_python_val(
weight_expr, ctx.env.schema)
except staeval.UnsupportedExpressionError:
raise errors.InvalidValueError(
f"fts::search weight_category must be a constant",
span=weight_expr.span,
)
weight_const = irutils.as_const(weight_expr)
if weight_const:
weight = str(weight_const.value)
else:
weight = None
assert weight
) from None

return irast.FTSDocument(
text=call.args[0].expr,
Expand Down
11 changes: 0 additions & 11 deletions edb/ir/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -498,17 +498,6 @@ def flt(n: irast.Call) -> bool:
return bool(ast.find_children(ir, irast.Call, flt, terminate_early=True))


def as_const(ir: irast.Base) -> Optional[irast.BaseConstant]:
match ir:
case irast.BaseConstant():
return ir
case irast.TypeCast():
return as_const(ir.expr)
case irast.SetE() if ir.expr:
return as_const(ir.expr)
return None


T = TypeVar('T')


Expand Down
36 changes: 36 additions & 0 deletions tests/test_edgeql_fts_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -555,3 +555,39 @@ async def test_edgeql_fts_schema_weight_03(self):
{"a": "hello world", "b": "running fox"},
],
)

async def test_edgeql_fts_schema_fiddly_args_01(self):
await self.con.execute(
r'''
create type Doc {
create required property x -> str;
create index fts::index on (
fts::with_options(
.x,
language := <fts::Language>('en'++'g'),
weight_category := (select fts::Weight.B),
)
);
};
'''
)

async def test_edgeql_fts_schema_fiddly_args_02(self):
async with self.assertRaisesRegexTx(
edgedb.InvalidValueError,
"fts::search weight_category must be a constant",
):
await self.con.execute(
r'''
create type Doc {
create required property x -> str;
create index fts::index on (
fts::with_options(
.x,
language := fts::Language.eng,
weight_category := <fts::Weight>("AB"[0]),
)
);
};
'''
)

0 comments on commit 7d8c4e4

Please sign in to comment.