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

Query builder: `GROUP`

Open colinhacks opened this issue 2 years ago • 4 comments

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

  • No way to specify "inline shape", e.g. group Movie { title } by .release_year.
  • The using is implicit and by doesn't appear syntactically

Naming Considerations

  • e.group.set could be e.grouping_set or just e.set
  • e.group.tuple could be e.grouping_list or just e.tuple (would require some special handling in toEdgeQL)
  • e.cube/e.rollup could be scoped to e.group.cube/e.group.rollup

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.

  • Pro: closer to EdgeQL in structure
  • Con: more verbose for simple cases. two levels of object nesting, even in the (likely quite common) case where no shape is needed. This shape can be added downstream in a wrapping e.select anyway.

colinhacks avatar May 10 '22 01:05 colinhacks

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.

vpetrovykh avatar May 11 '22 19:05 vpetrovykh

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.

colinhacks avatar May 11 '22 22:05 colinhacks

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".

vpetrovykh avatar May 11 '22 22:05 vpetrovykh

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.

elprans avatar May 14 '22 02:05 elprans

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

scotttrinh avatar May 02 '23 16:05 scotttrinh