edgedb / edgedb-js

The official TypeScript/JS client library and query builder for EdgeDB
https://edgedb.com
Apache License 2.0
514 stars 65 forks source link

Query builder: `GROUP` #346

Closed colinhacks closed 1 year ago

colinhacks commented 2 years ago

Proposed query builder API for group:

In EdgeQL:

group Movie
using 
  cast_size := count(.actors),
  first_letter := movie.title[0]
by .release_year;

Query builder:

// group Movie
e.group(e.Movie, movie => {

  // using 
  //    cast_size := count(.actors),
  //    first_letter := movie.title[0]
  const release_year = movie.release_year;
  const cast_size = e.count(movie.actors);
  const first_letter = movie.title[0];

  // by .release_year (equivalent)
  return {release_year}

  // by .release_year
  return movie.release_year;   // overload to support returning a plain property path

  // by .release_year, cast_size;
  return {release_year, cast_size}

  // by { .release_year, cast_size };
  return e.group.set({ release_year, cast_size })

  // by .release_year, { cast_size, first_letter }
  return {release_year,  ...e.group.set({cast_size, first_letter })}

  // by {.release_year, (cast_size, first_letter)}
  return e.group.set({release_year, ...e.group.tuple({c, d}))

  // by rollup(.release_year, cast_size, first_letter)
  return e.rollup({release_year, cast_size, first_letter})

  // by cube(.release_year, cast_size, first_letter)
  return e.cube({release_year, cast_size, first_letter})
})

This uses an object literal to simulate the fact that <grouping-ref> only accepts pre-defined aliases or simple paths. We need some string value to associate with each grouping-ref. The keys of the object literal correspond to the value of grouping in the final result. We should be able to repurpose some of our existing with extraction logic to properly add the necessary clauses to the using clause.

e.group.set and e.group.tuple wrap their input in an object with a single autogenerated key with a known prefix. In toEdgeQL, when a key with that prefix is encountered, the contents of the object it points to will be rendered inside curly braces or parens (respectively). The prefixed key itself won't occur anywhere in the resulting EdgeQL.

e.group.set({release_year});
// => { "grouping_set_a8b32d": {release_year} }

Downsides

Naming Considerations

Alternative: by key

// group Movie {title} by .release_year
e.group(e.Movie, movie => {
  return {
    title: true,
    by: <same as above>
  }
})

The by clause is specified in a special key called by. The rest of the object can be used to specify the shape to be applied to the elements.

vpetrovykh commented 2 years ago

I think it's OK to not have an inline shape for group in query builder. It is sugar for being wrapped in a select and conceptually it's probably simpler to think of grouping and then shaping the data as separate concerns. So I think that the first option looks fine.

I assume that CUBE and ROLLUP would be something like e.group.cube and e.group.rollup.

colinhacks commented 2 years ago

Indeed! Just updated the proposal to include those. I think top-level e.cube and e.rollup are ok though. The reason I scoped e.group.set and e.group.tuple is because they're not quite the same as regular e.set and e.tuple and it would increase implementation difficulty to overload those functions further.

vpetrovykh commented 2 years ago

I think it's nice to scope all of them under e.group because these constructs are exclusively used in group and nowhere else. They aren't types, they aren't functions, they are "grouping expressions", much like we have "type expressions" and plain "expressions".

elprans commented 2 years ago

Agree with @vpetrovykh re scoping of tools under e.group. Option 1 seems fine to me, as the loss of immediate shape is easily alleviated by composing a e.select.

scotttrinh commented 1 year ago

https://www.edgedb.com/docs/clients/js/group