Skip to content

Commit

Permalink
Don't bother creating SQL operators for edgeql operators (#7432)
Browse files Browse the repository at this point in the history
Just call the functions or underlying operators instead.
This should help a bit with #6697.
  • Loading branch information
msullivan authored Jun 6, 2024
1 parent e59a93f commit 22d06be
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 146 deletions.
2 changes: 1 addition & 1 deletion edb/buildmeta.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
# The merge conflict there is a nice reminder that you probably need
# to write a patch in edb/pgsql/patches.py, and then you should preserve
# the old value.
EDGEDB_CATALOG_VERSION = 2024_05_23_00_00
EDGEDB_CATALOG_VERSION = 2024_06_05_00_00
EDGEDB_MAJOR_VERSION = 6


Expand Down
8 changes: 5 additions & 3 deletions edb/edgeql/compiler/func.py
Original file line number Diff line number Diff line change
Expand Up @@ -632,9 +632,11 @@ def compile_operator(

from_op = oper.get_from_operator(env.schema)
sql_operator = None
if (from_op is not None and oper.get_code(env.schema) is None and
oper.get_from_function(env.schema) is None and
not in_polymorphic_func):
if (
from_op is not None
and oper.get_code(env.schema) is None
and oper.get_from_function(env.schema) is None
):
sql_operator = tuple(from_op)

origin_name: Optional[sn.QualName]
Expand Down
6 changes: 6 additions & 0 deletions edb/lib/std/25-numoperators.edgeql
Original file line number Diff line number Diff line change
Expand Up @@ -1751,6 +1751,7 @@ std::`+` (l: std::bigint, r: std::bigint) -> std::bigint {
CREATE ANNOTATION std::description := 'Arithmetic addition.';
SET volatility := 'Immutable';
SET commutator := 'std::+';
SET force_return_cast := true;
USING SQL OPERATOR r'+(numeric,numeric)';
};

Expand Down Expand Up @@ -1817,6 +1818,7 @@ std::`+` (l: std::bigint) -> std::bigint {
CREATE ANNOTATION std::identifier := 'plus';
CREATE ANNOTATION std::description := 'Arithmetic addition.';
SET volatility := 'Immutable';
SET force_return_cast := true;
USING SQL OPERATOR r'+(,numeric)';
};

Expand Down Expand Up @@ -1883,6 +1885,7 @@ std::`-` (l: std::bigint, r: std::bigint) -> std::bigint {
CREATE ANNOTATION std::identifier := 'minus';
CREATE ANNOTATION std::description := 'Arithmetic subtraction.';
SET volatility := 'Immutable';
SET force_return_cast := true;
USING SQL OPERATOR r'-(numeric,numeric)';
};

Expand Down Expand Up @@ -1948,6 +1951,7 @@ std::`-` (l: std::bigint) -> std::bigint {
CREATE ANNOTATION std::identifier := 'minus';
CREATE ANNOTATION std::description := 'Arithmetic subtraction.';
SET volatility := 'Immutable';
SET force_return_cast := true;
USING SQL OPERATOR r'-(,numeric)';
};

Expand Down Expand Up @@ -2019,6 +2023,7 @@ std::`*` (l: std::bigint, r: std::bigint) -> std::bigint {
CREATE ANNOTATION std::description := 'Arithmetic multiplication.';
SET volatility := 'Immutable';
SET commutator := 'std::*';
SET force_return_cast := true;
USING SQL OPERATOR r'*(numeric,numeric)';
};

Expand Down Expand Up @@ -2359,6 +2364,7 @@ std::`^` (n: std::bigint, p: std::bigint) -> std::decimal {
CREATE ANNOTATION std::identifier := 'pow';
CREATE ANNOTATION std::description := 'Power operation.';
SET volatility := 'Immutable';
SET force_return_cast := true;
USING SQL OPERATOR '^(numeric,numeric)';
};

Expand Down
16 changes: 4 additions & 12 deletions edb/pgsql/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ def get_pointer_backend_name(id, module_name, *, catenate=False, aspect=None):
return convert_name(name, suffix=suffix, catenate=catenate)


_operator_map = {
operator_map = {
s_name.name_from_string('std::AND'): 'AND',
s_name.name_from_string('std::OR'): 'OR',
s_name.name_from_string('std::NOT'): 'NOT',
Expand All @@ -336,20 +336,12 @@ def get_operator_backend_name(name, catenate=False, *, aspect=None):
raise ValueError(
f'unexpected aspect for operator backend name: {aspect!r}')

oper_name = _operator_map.get(name)
oper_name = operator_map.get(name)
if oper_name is None:
oper_name = name.name
if re.search(r'[a-zA-Z]', oper_name):
# Alphanumeric operator, cannot be expressed in Postgres as-is
# Since this is a rare occasion, we hard-code the translation
# table.
if oper_name == 'OR':
oper_name = '|||'
elif oper_name == 'AND':
oper_name = '&&&'
else:
raise ValueError(
f'cannot represent operator {oper_name} in Postgres')
raise ValueError(
f'cannot represent operator {oper_name} in Postgres')

oper_name = f'`{oper_name}`'
schema = 'edgedb'
Expand Down
32 changes: 18 additions & 14 deletions edb/pgsql/compiler/expr.py
Original file line number Diff line number Diff line change
Expand Up @@ -481,18 +481,30 @@ def compile_operator(
assert rexpr
result = pgast.NullTest(arg=rexpr, negated=True)

elif expr.func_shortname in common.operator_map:
sql_oper = common.operator_map[expr.func_shortname]

elif expr.sql_operator:
sql_oper = expr.sql_operator[0]
if len(expr.sql_operator) > 1:
# Explicit operand types given in FROM SQL OPERATOR
lexpr, rexpr = _cast_operands(lexpr, rexpr, expr.sql_operator[1:])

elif expr.sql_function:
sql_func = expr.sql_function[0]
func_name = tuple(sql_func.split('.', 1))
if len(expr.sql_function) > 1:
# Explicit operand types given in FROM SQL FUNCTION
lexpr, rexpr = _cast_operands(lexpr, rexpr, expr.sql_function[1:])
elif expr.origin_name is not None:
sql_oper = common.get_operator_backend_name(
expr.origin_name)[1]

else:
if expr.sql_function:
sql_func = expr.sql_function[0]
func_name = tuple(sql_func.split('.', 1))
if len(expr.sql_function) > 1:
# Explicit operand types given in FROM SQL FUNCTION
lexpr, rexpr = _cast_operands(
lexpr, rexpr, expr.sql_function[1:])
else:
func_name = common.get_operator_backend_name(
expr.func_shortname, aspect='function')

args = []
if lexpr is not None:
Expand All @@ -502,14 +514,6 @@ def compile_operator(

result = pgast.FuncCall(name=func_name, args=args)

elif expr.origin_name is not None:
sql_oper = common.get_operator_backend_name(
expr.origin_name)[1]

else:
sql_oper = common.get_operator_backend_name(
expr.func_shortname)[1]

# If result was not already computed, it's going to be a generic Expr.
if result is None:
result = pgast.Expr(
Expand Down
136 changes: 20 additions & 116 deletions edb/pgsql/delta.py
Original file line number Diff line number Diff line change
Expand Up @@ -1740,16 +1740,11 @@ def apply(
oper_fromop = oper.get_from_operator(schema)
oper_fromfunc = oper.get_from_function(schema)
oper_code = oper.get_code(schema)
oper_comm = oper.get_commutator(schema)
if oper_comm:
commutator = self.oper_name_to_pg_name(schema, oper_comm)
else:
commutator = None
oper_neg = oper.get_negator(schema)
if oper_neg:
negator = self.oper_name_to_pg_name(schema, oper_neg)
else:
negator = None

# We support having both fromop and one of the others for
# "legacy" purposes, but ignore it.
if oper_code or oper_fromfunc:
oper_fromop = None

if oper_language is ql_ast.Language.SQL and oper_fromop:
pg_oper_name = oper_fromop[0]
Expand All @@ -1760,103 +1755,29 @@ def apply(
else:
from_args = args

if oper_code:
oper_func = self.make_operator_function(oper, schema)
self.pgops.add(dbops.CreateFunction(oper_func))
oper_func_name = common.qname(*oper_func.name)

elif oper_fromfunc:
oper_func_name = oper_fromfunc[0]
if len(oper_fromfunc) > 1:
from_args = oper_fromfunc[1:]

elif from_args != args:
# Need a proxy function with casts
oper_kind = oper.get_operator_kind(schema)

if oper_kind is ql_ft.OperatorKind.Infix:
op = (f'$1::{from_args[0]} {pg_oper_name} '
f'$2::{from_args[1]}')
elif oper_kind is ql_ft.OperatorKind.Postfix:
op = f'$1::{from_args[0]} {pg_oper_name}'
elif oper_kind is ql_ft.OperatorKind.Prefix:
op = f'{pg_oper_name} $1::{from_args[1]}'
else:
raise RuntimeError(
f'unexpected operator kind: {oper_kind!r}')

rtype = self.get_pgtype(
oper, oper.get_return_type(schema), schema)

oper_func = dbops.Function(
name=common.get_backend_name(
schema, oper, catenate=False, aspect='function'),
args=[(None, a) for a in args if a],
volatility=oper.get_volatility(schema),
returns=rtype,
text=f'SELECT ({op})::{qt(rtype)}',
)

self.pgops.add(dbops.CreateFunction(oper_func))
oper_func_name = common.qname(*oper_func.name)

else:
oper_func_name = None

if (
pg_oper_name is not None
and not params.has_polymorphic(schema)
or all(
p.get_type(schema).is_array()
for p in params.objects(schema)
)
and not oper.get_force_return_cast(schema)
):
self.pgops.add(dbops.CreateOperatorAlias(
name=common.get_backend_name(schema, oper, catenate=False),
args=args,
procedure=oper_func_name,
base_operator=('pg_catalog', pg_oper_name),
operator_args=from_args,
commutator=commutator,
negator=negator,
))

if not params.has_polymorphic(schema):
if oper_func_name is not None:
cexpr = self.get_dummy_func_call(
oper, oper_func_name, schema)
else:
cexpr = self.get_dummy_operator_call(
oper, pg_oper_name, from_args, schema)

# We don't do a strictness consistency check for
# USING SQL OPERATOR because they are heavily
# overloaded, and so we'd need to take the types
# into account; this is doable, but doesn't seem
# worth doing since the only non-strict operator
# is || on arrays, and we use array_cat for that
# anyway!
check = self.sql_rval_consistency_check(
oper, cexpr, schema)
self.pgops.add(check)
elif oper_func_name is not None:
self.pgops.add(dbops.CreateOperator(
name=common.get_backend_name(schema, oper, catenate=False),
args=from_args,
procedure=oper_func_name,
))
cexpr = self.get_dummy_operator_call(
oper, pg_oper_name, from_args, schema)

# We don't do a strictness consistency check for
# USING SQL OPERATOR because they are heavily
# overloaded, and so we'd need to take the types
# into account; this is doable, but doesn't seem
# worth doing since the only non-strict operator
# is || on arrays, and we use array_cat for that
# anyway!
check = self.sql_rval_consistency_check(
oper, cexpr, schema)
self.pgops.add(check)

elif oper_language is ql_ast.Language.SQL and oper_code:
args = self.get_pg_operands(schema, oper)
oper_func = self.make_operator_function(oper, schema)
self.pgops.add(dbops.CreateFunction(oper_func))
oper_func_name = common.qname(*oper_func.name)

self.pgops.add(dbops.CreateOperator(
name=common.get_backend_name(schema, oper, catenate=False),
args=args,
procedure=oper_func_name,
))

if not params.has_polymorphic(schema):
cexpr = self.get_dummy_func_call(
Expand Down Expand Up @@ -1907,24 +1828,7 @@ class AlterOperator(OperatorCommand, adapts=s_opers.AlterOperator):


class DeleteOperator(OperatorCommand, adapts=s_opers.DeleteOperator):
def apply(
self,
schema: s_schema.Schema,
context: sd.CommandContext,
) -> s_schema.Schema:
orig_schema = schema
oper = schema.get(self.classname, type=s_opers.Operator)

if oper.get_abstract(schema):
return super().apply(schema, context)

name = common.get_backend_name(schema, oper, catenate=False)
args = self.get_pg_operands(schema, oper)

schema = super().apply(schema, context)
if not oper.get_from_expr(orig_schema):
self.pgops.add(dbops.DropOperator(name=name, args=args))
return schema
pass


class CastCommand(MetaCommand):
Expand Down

0 comments on commit 22d06be

Please sign in to comment.