edgedb-js
edgedb-js copied to clipboard
Implement variadic version of `e.op`
Currently, e.op
is either unary, binary, or ternary and there is no easy way to combine a variadic number of operands using the same operator. This is frequently needed to, e.g., combine a bunch of filter expressions into an and
chain.
I propose adding e.gather_op('<op>', [operands])
as a way to do that. For example:
const selectionSet = {title: true} as const;
const query = e.select(e.Movie, movie => {
const filters = [
e.op(movie.title, "ilike", "%avengers%"),
];
if (genre) {
filters.push(e.op(movie.genre, "in", e.Genre.RomCom));
}
return {
...selectionSet,
filter: e.gather_op("and", filters),
};
});
const result = await query.run(client);
@colinhacks also suggests adding a variadic e.and()
sugar, because this is a very common pattern.
apply_op
is closer to JS idioms.
The issue here is that there's a difference between what all()
does and what e.gather_op('and', ...)
would do.
-
all()
is a semantic construct, that extends the booleanand
operator to a set. It's part of EdgeQL. -
e.gather_op()
is a syntactic construct. It literally is a shorthand for the syntax of writing a bunch of expressions connected withand
. It's barely more sophisticated than'(' + expsrs_list.join(') and (') + ')'
. Alternatively you could view it as an ad-hoc element-wise function, but since we don't have those in EdgeQL, the construct is still firmly in the realm of query-builder, not EdgeQL itself. It is syntax sugar that currently only exists outside of EdgeQL. (Adding it to EdgeQL is tricky because it would introduce subtle confusion between thismulti and
andall
that may be a bigger problem than the one it solves)
I'm against sugar (e.and()
) on top of sugar (generic e.gather_op()
) as that tends to make the API too complicated and heterogeneous.
apply
isn't exactly right, because we don't "apply" and
to each argument, it's more like "join".
I'm against sugar (
e.and()
) on top of sugar
I'm also a bit wary of adding e.and()
sugar right away. IMO we should stick to the "generic first, sugar later" approach.
apply
isn't exactly right, because we don't "apply"and
to each argument, it's more like "join".
Maybe we should call it e.join_op()
to emphasize that this is basically syntactically stitching stuff with an operator?
Oh, wait... maybe "join" is a bad and confusing keyword in the database context.
Oh, wait... maybe "join" is a bad and confusing keyword in the database context.
Exactly.
Perhaps e.op_all
or e.op_many
?
Agreed w.r.t e.and
- retracted.
I would avoid all
in the name for this construct to distinguish it from the all
builtin as much as possible.
Haven't seen any suggestion for e.combine_op
yet. Or maybe rather e.op_combine
for autosuggest closeness.
Also how about pluralizing it? e.combine_ops
?
I came here because the EdgeDB AI docs suggested me to use e.and([e.op(), e.op(), e.op()])
.
After a little more prompting, it told me that e.and()
did in fact not exist 😅
Although it would have been great for my many nested e.op(...,'and',...)
@dotlouis
FWIW, we do have something close to this in the form of e.all
which would look like:
e.all(e.set([e.op(), e.op(), e.op()]));
Which uses the all
standard library function to chain a set of booleans together.
all({a, b, c})
This is different from the proposed issue here which is wanting generate:
(a and b and c)
Stay tuned as we work on trying to pick up on some of these ergonomic issues.
Hi @scotttrinh, thanks. I did not think of this solution. It's neat, but I guess it's not very intuitive. So maybe having helper functions or aliases, (at least in the doc) would be helpful
@dotlouis There is some existing guidance in the "Filtering" section of the client docs here: https://www.edgedb.com/docs/clients/js/select#filtering
Also, there is some similarity here between TypeScript/JavaScript's &&
/||
operators and the Array.prototype#every
/Array.prototype#some
, so hopefully that gives some further intuition here if you're familiar with how those array functions work.
RE: helper functions and aliases
We've tried to keep the query builder syntax as close as possible to GraphQL to make learning one give some intuition about the other, so we've avoided having constructs in the query builder that are sugar for existing patterns. But indeed, the topic being discussed here is exactly that sort of thing: can we introduce some useful functions to make some common patterns more ergonomic.
You're right the doc is perfectly clear. I guess the original issue is that I got recommended a e.and()
method that did not exist from the AI chat-bot while it should have pointed me to the above solution.