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

Simplified cardinality inference

Open colinhacks opened this issue 1 year ago • 1 comments

The proposals here are compatible with https://github.com/edgedb/edgedb-js/issues/347: "object filter syntax"

e.select(e.Movie, ()=>({
  // exclusive prop
  filter: { id: "abcd..." }

  // composite exclusive prop
  filter: { title: "Iron Man", release_year: "2008" }
}));

There are a few problems with cardinality inference and the DX of e.select currently.

  1. Cardinality inference doesn't work with composite exclusive constraints
  2. It is verbose to write simple select-by-id or select-by-exclusive-property queries
  3. There is a lot of type infrastructure required to perform card inference that may contribute to TypeScript perf problems experienced by some users

There are two main proposals for addressing this:

1. filter_single key

Alt name: single — which is both shorter and a reserved keyword.

Passing any boolean expression to filter_single will result in a singleton expression.

e.select(e.Movie, (movie)=>({
  filter_single: e.bool(true)
  title: true
}));
// { title: string } | null

This proposal is compatible with "object filter" syntax.

e.select(e.Movie, ()=>({
  // exclusive prop
  filter_single: { id: "abcd..." }

  // given: constraint exclusive on (.title, .release_year)
  filter_single: { title: "Iron Man", release_year: "2008" }
}));

This obviates the need for e.op to perform simple singleton queries. We are providing a special key to indicate clearly that a query returns a singleton; it also fixes the API inconsistency associated with overloading filter.

Exclusive constraints associated with more complex EdgeQL expressions are beyond the scope of this proposal.

2. e.select_single

A new top-level statement that always infers a singleton cardinality. The filter key would accept an arbitrary boolean expression.

e.select_single(e.Movie, (movie)=>({
  // exclusive prop
  filter: { id: "abcd..." },

  // composite exclusive prop
  filter: { title: "Iron Man", release_year: "2008" },

  // composite exclusive prop
  filter: <any boolean expression>
}));

A top-level select_single introduces some weirdness. Does every e.select return an expression with cardinality many, similar to calling client.query()? If so this is a little odd:

e.str('adsf').run(client); // => string
e.select(e.str('adsf')).run(client); // => string[]

To address this e.select could simply "pass through" the cardinality of its first argument. So wrapping a singleton expression in e.select would return another singleton. In other words:

  • e.select doesn't change the cardinality of it's argument
  • e.select_single sets an upper bound of 1

This is consistent and easy to explain while maintaining expected behavior, though it's arguably inconsistent with the behavior of query and querySingle methods.


Backwards compatibiility

A totally unbiased breakdown of the arguments for and against breaking backwards.

  • For breaking:
    • There is a lot of type complexity that could be stripped out if we break compatibility and remove the old inference logic.
    • We can also "flatten" all expressions and dramatically reduce load on the TypeScript compiler, since we'll no longer need to track a static representation of the entire expression tree.
    • We can release this change as part of a breaking release in conjunction with the switch to npx @edgedb/generate.
    • The migration would consists only of changing "filter" to "filter_single" wherever singleton cardinality is expected.
    • We're still in v0.x so all minor versions should be considered potentially breaking by users anyway.
  • Against breaking: A small minority of users will be briefly annoyed

colinhacks avatar Aug 10 '22 00:08 colinhacks

e.select(e.Movie, (movie)=>({ filter_single: e.bool(true) title: true })); // { title: string } | null

Does the existence of a filter_single key mean we implicitly wrap the select with assert_single?

jaclarke avatar Sep 20 '22 19:09 jaclarke

e.select(e.Movie, (movie)=>({ filter_single: e.bool(true) title: true })); // { title: string } | null

Does the existence of a filter_single key mean we implicitly wrap the select with assert_single?

I think so, yeah

colinhacks avatar Sep 27 '22 16:09 colinhacks

I think the title of the issue should be changed (or separated) to reflect that, as stated in the body, the issues are not just about DX but also incorrectness of cardinality inference in common workflows (even checking a unique username/mail and a password), which may better signal the urgency of the situation

avedetvedea avatar Oct 08 '22 23:10 avedetvedea