Implemented one-off query support for TypeScript SDK

[egormanga]

Description of Changes

Was necessary (at least) because of #1599.

API and ABI breaking changes

None(?)

Needs docs, that's for certain.

Expected complexity level and risk

4:

Doesn't seem to break anything, but who knows. I'm not sure about this one.

Also, performance impact? It should only get better, but again I have no info on that.

Testing

• Manual testing
• Use case fit
• Unit tests

[gefjon]

Thanks so much for your contribution! This is a feature that we're very interested in merging. As discussed in comments below, we'd prefer to have the TypeScript client SDK expose the same interface for remote queries as the C# client SDK. That interface is:

Each TableHandle returned by the methods on ctx.db has a method remoteQuery. That method looks roughly like:

class TableHandleImpl<Row> {
  async remoteQuery(filters: string): Row[] {
    const tableName = this.tableName();
    const fullQuery = `SELECT ${tableName}.* FROM {tableName} ${filters}`;
    // Construct a promise, register it in the `DbConnection`, send a `OneOffQuery` message, return the promise.
    // When the `DbConnection` receives the response, it parses the rows and fulfills the promise with them.
  }
}

Client code might invoke this like:

async getPlayersInFaction(ctx, factionId): Player[] {
  const interestingPlayers = await ctx.db.player().remoteQuery(`WHERE faction = ${factionId}`);
  return interestingPlayers;
}

[gefjon]

The API we use in the C# SDK for running remote queries has them as methods of the TableHandle, rather than on the DbContext. This provides some amount of type safety, as the method inserts the SELECT clause into the query and also encodes the TypeScript type of the rows returned.

[gefjon]

Also, we use the phrase "remote query" to describe this operation in the user-facing API. This emphasizes that this is a query that bypasses the client cache and goes directly to the remote database.

[egormanga]

At the same time, formatting SELECTs restricts the provided functionality to querying only. What if a client needs to execute something else, e.g. INSERT, perhaps with the owner's identity?

[egormanga]

Okay, there I discovered (in #2968) that SpacetimeDB does currently not support anything other than SELECT even in one-shot queries.

[gefjon]

Correct, we have no interest in generalizing one-off queries to support SQL DML operations like INSERT, DELETE, UPDATE, &c.

[gefjon]

Remote queries are an odd and mismatched part of our API. Rather than using a builder and callbacks, they are async methods which directly return the queried rows. What you've designed here would be better, in that it more closely matches the way we construct subscriptions. Unfortunately, though, the async version is a part of the C# client SDK's stable API, and we are committed to continuing to support it. We're also committed to minimizing differences between the SDKs, which means that TypeScript should expose the same interface.

[egormanga]

To be perfectly honest, async remote queries seem to be a more intuitive and straightforward solution, even though they differ from the subscriptions. Being able to use queries anywhere in the code without registering a callback is pretty convenient.

I see your appreciation and in that regard will surely replace the current implementation with a table method. On the other hand, might it be useful to keep both, e.g. even providing the builder version across all implementations?

[egormanga]

The main showstopper for your proposed approach is the fact that the entire SDK is currently callback-based, thus rendering it pretty impossible to implement an async querying mechanism in a useful fashion.

For instance, if I need to fire a query inside a subscription callback, I won't be able to await for it inside that block. Since I'd need to wrap it in a Promise anyway, similar to the current API, why bother implementing this at all?

[gefjon]

On the other hand, might it be useful to keep both, e.g. even providing the builder version across all implementations?

We'd need a more involved design process in order to make this happen. Going through such a design process with someone external to Clockwork Labs poses a number of problems, and seems unlikely to be pleasant for anyone involved.

if I need to fire a query inside a subscription callback, I won't be able to await for it inside that block.

Correct, you'll instead do ctx.db.myTable().remoteQuery("WHERE whatever").then(rows => processRows(rows));.

[egormanga]

Sure, you may design the thing as you wish (or, may not at all). I'll look into the possibility of refactoring the implementation to not depend on this custom solution (and it's clearly possible, but however be the cleanest wins).

[egormanga]

The builder showed to be more convenient even in the internal code, so I'm suggesting keeping it.

[gefjon]

This doesn't seem possible inside loops, though.

I don't understand why being inside a loop would be in any way different.

[egormanga]

Say you need to fetch some relation for each row (basically, .map() them) and do something with the resulting array. You do a query, .then() on it, and inside that you iterate over the rows. On each iteration, you issue another query, and that's where you only can await it and then use Promise.all() on the whole .map(), otherwise it's a callback inside a loop and that's not possible without refactoring.

Example for my API (assume inside a promise):

ctx.subscriptionBuilder()
    .onApplied(ctx => {
        ctx.queryBuilder()
            .onResolved((ctx, tables) => {
                const messages = tables.get('message');
                resolve(messages?.iter().map(message => {
                    const user = ctx.db.user.id.find(message.authorId);
                    return {user, message};
                });
            })
            .query(`SELECT * FROM message;`);
    })
    .subscribe(`SELECT * FROM user;`);

Examples for your API:

ctx.subscriptionBuilder()
    .onApplied(ctx => {
        const messages = await ctx.db.message.remoteQuery(`SELECT * FROM message;`);
        //               ^^^^^ error: await inside non-async function
        resolve(…);
    })
    .subscribe(`SELECT * FROM user;`);

ctx.subscriptionBuilder()
    .onApplied(async ctx => {
        //     ^^^^^ sure possible, but who will ever await it?
        const messages = await ctx.db.message.remoteQuery(…);
        resolve(…);  // will never get called
    })
    .subscribe(…);

[gefjon]

Your second example:

ctx.subscriptionBuilder()
  .onApplied(ctx => {
    ctx.db.message.remoteQuery("WHERE foo = bar")
      .then(messages => {
        resolve(doStuffWith(messages));
      });
  )
  .subscribe(`SELECT * FROM user;)`;

Your first example:

ctx.subscriptionBuilder()
  .onApplied(ctx => {
    ctx.db.message.remoteQuery("")
      .then(messages => {
        resolve(messages.iter().map(message => {
          const user = ctx.db.user.id.find(message.authorId);
          return { user, message };
        });
      });
  .subscribe(`SELECT * FROM user;`);

Keep in mind that in JS, promises are always run by the executor, and async/await is syntax sugar over the then callbacks. This is unlike e.g. Rust, where promises famously do nothing unless awaited.

[egormanga]

Alright, you convinced me, thank you. Let me make the API private in a sec.

[egormanga]

It turned out to be a really convenient implementation of an async ctx.table.remoteQuery() using the builder I already implemented. Done just that.

[gefjon]

I would prefer not to expose your query builder as a new public user-facing API. Even if it's not documented, people will inevitably stumble upon it and use it in their code. This will have the twin problems of exposing them to errors like the one you saw in #2968 , and forcing us to maintain the new API in a compatible way in order to preserve their code. To avoid this, I recommend moving it from a class method to a free function, and making sure said free function is not exported from the package.

[jsdt]

This will be a problem after 255 queries. Using a 4 byte integer would probably be safe enough, but we would need to convert it to bytes with something like:

const queryId = this.#getNextQueryId();
const buffer = new ArrayBuffer(4);
const view = new DataView(buffer);
view.setUint32(0, queryId, true); // true = little-endian

[egormanga]

Sure, thanks for spotting it! I added a single-byte counter as a placeholder and totally forgot to widen it.

[jsdt]

I don't think we want to call the row-level callbacks for one-off queries, since any rows that are inserted here will never have corresponding delete callbacks.

Since we aren't calling those, we don't need to have the #applyTableState function. We can just get a list of rows for each table.

[egormanga]

I agree we can remove the callbacks. But applyTableState() processes the insertion events from the response, isn't it necessary to compute the rows?