@aboodman realized this while implementing this. It actually can be done without changing the Change api.

Note from video review with Aaron:

It seems fine to have the subquery filter subqueries in the output view, just not in the TypeScript type. It may even help with debugging/understanding. Suggests not doing this hidden format for now.

This enhancement to take should land first as a standalone change.

Should this be
correlatedSubqueryConditionConditionSchema
or
correlatedSubqueryConditionOperatorSchema

for EXISTS and NOT EXISTS there are no other operands other than the subquery, but for other subquery conditions there may be additional operands.

I think this should be an operator. It is more symmetrical with SimpleCondition that way.

type Condition = SimpleCondition | Cojunction | Discjunction | CorrelatedSubqueryCondition;

type CorrelatedSubqueryCondition = {
  type: 'correlatedSubquery';
  related: CorrelatedSubquery;
  op: 'EXISTS' | 'NOT EXISTS'
};

How should we name these for subquery filters, and how should we fix the naming collision issue for all subqueries?

Each whereExists gets an incrementing value appended to its name? The user never needs to interact with the results of a whereExists relationships so we can generate any name we like for them.

related calls should be able to take an alias parameter. If related calls collide, that's on the developer to deal with.

We need some way to avoid collision between the user's aliases and the ones we create for subquery filters.

Reserved character or prefix?

An alternative design is for push to always fetch the relationship with code like:

    const relationship = must(
      first(
        this.#input.fetch({
          start: {row: node.row, basis: 'at'},
        }),
      ),
    ).relationships[this.#relationshipName]

and to then check the relationships size.

This alternative approach is simpler, stateless, and it also generalizes to support other subquery condition types, it could allow implementing a general SubqueryFilter operator like:

export class SubqueryFilter implements Operator {
  constructor(
    input: Input,
    storage: Storage,
    relationshipName: string,
    prediate: (relationship: Stream<Node>) => boolean
  ) 

The downside is it is less efficient.

We need a better API for this at the Query level, something that supports subquery conditions nested in not/and/or conditions.

Perhaps the ideas in the "or API improvements" bug can apply here https://bugs.rocicorp.dev/issue/3104

let q = z.issue.where({cmp, or, and, not}=> or(
    filters.map(f => cmp('category', f))
));

could be extend to have:

let q = z.issue.where({cmp, or, and, not, exists}=> or(
    filters.map(f => cmp('category', f))
));

Yeah, the latter is also how Kysely handles exists: https://kysely.dev/docs/recipes/expressions#expression-builder

Good to know the path is well trodden there. I'm in favor of that form.

I know @aboodman was hoping to remove the nomenclature of exists and notExists entirely. That we'd instead do something like:

z.issue.related('comments').where('comments.length', '>', 0)

Or an alternative formulation:

z.issue.where(
  related('comments').length, '>', 0
);

That's too much for me to entertain at the moment without making it my main focus. At first glance, an API of that form greatly expands the surface area (in terms of features) that we would need to cover. It pulls in aggregating over the subquery then comparing rather than simple existence checks.

I think we should do exists and non-exist for now and unblock read permissions.

yay! hidden should have been a format param. I think it predated format though.

I can't seem to get the typing right here...

How do we avoid conflicts between selecting a subquery and filtering on one with the same name?

issue.related('creator').where(exists('creator'))

Can we reduce to doing a single join?

We could do this optimization here in the build step.

nit: seems like a random return.

do we not need to cleanup our own storage?

we need to re-fetch the row in order to populate relationships? Is that the issue?

Yes.
In this case a child change needs to be translated to an add of the parent (because now a child exists). Child changes only have a row, but an add needs a Node (i.e. a row and a tree of relationships).

do we want to store the entire size?? Or just up to some limit?

The subquery is limited in builder.ts, see

csq = {...csq, subquery: {...csq.subquery, limit: EXISTS_LIMIT}};

It is done there to also limit the number of rows we sync to the client.

rm

Still need to track down why my changes to the kitchen sync test caused this assert to start failing (even if the query has no exists/non-exists).

Each whereExists gets an incrementing value appended to its name? The user never needs to interact with the results of a whereExists relationships so we can generate any name we like for them.

related calls should be able to take an alias parameter. If related calls collide, that's on the developer to deal with.