edgedb-js icon indicating copy to clipboard operation
edgedb-js copied to clipboard

RFC: Simplified filtering API

Open colinhacks opened this issue 2 years ago • 6 comments

A proposal for simplified filter syntax for equality filters. The goal is to expand the scope of cardinality inference and provide a less verbose alternative to e.op(x, "=", y) when specifying simple equality filters.

In e.select, the filter key should also accept a plain JS object. The keys can correspond to an property/link name and can point to singleton expressions or plain JS literals.

Current syntax

e.select(e.Movie, (movie) => ({
  filter: e.op(movie.release_year, '=', '2008'),
}));

Proposed syntax

e.select(e.Movie, () => ({
  filter: {release_year: 1999},
}))

It's no longer necessary to include a reference to the scope variable movie or write the e.op() expression, which is one of the bits of query builder syntax that is hardest to type quickly and least conducive to autocompletion.

Type inference still works as expected. Filtering on a non-exclusive property returns an array:

e.select(e.Movie, (movie) => ({
  title: true,
  filter: {release_year: 1999},
})).run(client);
// {title: string}[]

If a referenced property/link has an exclusive constraint, the result will be a singleton.

e.select(e.Movie, (movie) => ({
  title: true,
  filter: {id: e.uuid('2053a8b4-49b1-437a-84c8-e1b0291ccd9f')},
}));
// {title: string}

This hold true for object-level exclusive constraints corresponding to a tuple of properties. (This is infeasible to implement using the e.op formulation.) e.g. if there is a compound constraint on (.title, .release_year) the following would be inferred to be a singleton.

// singleton
e.select(e.Movie, (movie) => ({
  title: true,
  filter: {
    title: "A Star is Born"
    release_year: 2018
  }
}));
// {title: string}

An additional proposal is to support cardinality inference for "expression constraints". e.g.

type Movie {
  property title -> str;
  constraint exclusive on str_lower(.title);
}

The constraint's stored expr would be introspected to support the following:

// singleton
e.select(e.Movie, (movie) => ({
  title: true,
  filter: {
    'str_lower(.title)': 'a star is born',
  },
}));

Non-exclusive

There is a question of whether the "object filter" syntax would support non-exclusive properties. @elprans objected to supporting this syntax exclusively for singleton/exclusive filtering. Filtering by equality on an exclusive vs nonexclusive property would look very different and possibly lead to confusion.

e.select(e.Movie, (,)=>({
  filter: { id: "adsf" }
  filter: e.op(m.rating, '=', 5),
}));

This proposal can be generalized to a way to specify any set of equality filters (https://github.com/edgedb/edgedb-js/issues/347)

e.select(e.Movie, ()=>({
  filter: { rating: 5 }  // non-exclusive prop
}));

This fixes the inconsistency, but makes it less visually obvious when a filter clause corresponds to a singleton filter. It also increases implementation complexity, since we'll need to statically determine whether keys of the "object filter" object correspond to an exclusive constraint, instead of simply checking if the value passed to filter is an object literal.

colinhacks avatar May 10 '22 03:05 colinhacks

Related as a further syntactic sugar: https://github.com/edgedb/edgedb/issues/3810#issuecomment-1119942917

When writing a subquery select-by-id, it would be great if we had an even more compact syntax to refer to the linked entity by its ID without spelling out the query.

liron00 avatar May 31 '22 21:05 liron00

What about other operators?

Personally, I prefer something close to Graphql's style:


FROM: filter: e.op(left, '>', right)

TO: filter: {left: {e.gt: right}}

FROM: filter: e.op(left, 'or', right)

TO: filter: {e.or: [left, right]}

...

ghost avatar Jun 02 '22 18:06 ghost

How would you combine new syntax and old if you have a bunch of eq filters and one non-eq? Would this be possible:

e.select(e.Movie, (movie) => ({
  title: true,
  filter: e.op({
    title: "A Star is Born"
    release_year: 2018
  }, 'and', e.op(movie.blah, '>', 10))
}));

?

Also, do we have to use e.uuid()? Can we allow ID types to accept string literals directly? The casting seems a bit too verbose and not super necessary.

1st1 avatar Jun 03 '22 20:06 1st1

Personally, I prefer something close to Graphql's style:

I don't hate this, it's familiar to people and has better autocompletion than e.op. But it also doesn't have the composability of e.op and requires several levels of object nesting, which I don't love. It's certainly on the table, but it's a separate discussion.

How would you combine new syntax and old if you have a bunch of eq filters and one non-eq? Would this be possible:

This syntax wouldn't be allowed under the current proposal since I'm not planning to overload e.op to accept these "filter objects". This is merely an extension to what can be passed into the filter expression in a select/update/delete. Previously you could only pass a boolean expression, now you can pass either a boolean expression or this special-case "object filter". I also don't think it's important for this syntax to be composable in e.op since it'll almost exclusively be used in conjunction with id or exclusive properties.


Both of these points make me think that my original proposal will be easier to explain/understand, namely that this "object filter" syntax should only work for exclusive properties or tuples.

@elprans argued in favor of expanding this syntax to include all properties, s.t. this is a general purpose syntactic sugar for specifying any equality filter. But personally I like the idea of a special syntax only for uniquely specifying a single object. It's makes it visually obvious that you are selecting a single object, which makes cardinality inference less mysterious. Providing this syntax to include all properties invites additional questions, including the two above: "How do I use other operators?" and "How do I compose this with other expressions?". Both of these potential points of confusion are eliminated if we limit to exclusive properties/tuples, since there's never a reason to use non-equality operators or do .id = "whatever" AND <expr> when selecting a single object. (Not for nothing, it's also would dramatically reduce implementation complexity.)

colinhacks avatar Jun 06 '22 19:06 colinhacks

do we have to use e.uuid()? Can we allow ID types to accept string literals directly? The casting seems a bit too verbose and not super necessary

We are doing this currently because we're being conservative and trying to keep the query builder semantics as close as possible to EdgeQL. Personally I agree this causes more confusion that it's worth.

colinhacks avatar Jun 06 '22 19:06 colinhacks

String IDs without e.uuid seems good IMO, might just want to clarify the deal with the string allowed to have / not have dashes

liron00 avatar Jun 06 '22 20:06 liron00

Is there something blocking this from getting into EdgeDB?

NetOpWibby avatar Sep 28 '22 23:09 NetOpWibby

Implemented in the query builder: https://www.edgedb.com/docs/clients/js/querybuilder#select-a-single-object

scotttrinh avatar May 03 '23 15:05 scotttrinh