Event Handlers: does not support promises about atdatabases HOT 2 OPEN

Another issue I expect you will run into is that the onQueryStart and onQueryResults are not passed the transaction/connection (or any way to run a query within the current transaction/connection). This means that even if you have the option to await, you will not be able to safely run any other db queries as they would run outside the current transaction context.

If you intend to asynchronously log to some external service, i.e. not Postgres, you wouldn't have this issue, but it would still not be a great idea for performance. At the point that onQueryStart is called, an exclusive lock on the database connection has been acquired to run that query. If we supported promises, this lock would not be released until after your async logging operation completes.

In my projects, I normally just log to stdout/stderr, via console.log/console.warn, and then parse that log in a separate process as needed (we use JSON in our logs for structured data). You could alternatively just log without waiting for the result, e.g.

function onQueryStart(_query, formattedQuery) {
  writeToRemoteLog(formattedQuery).catch(ex => {
    console.error(`Failed to write to log: ${ex.message}`)
  })
}

The reason for enforcing undefined was to ensure that if we wanted to add support for additional functionality in the future, we could do so without breaking changes. The features I can currently see that would make sense here are:

1. Returning a modified query from onQueryStart to run instead of the provided query. A good use case for this would be adding comments so that we can more easily trace the source of slow/expensive queries (e.g. https://google.github.io/sqlcommenter/spec/#format).
2. Aborting queries in onQueryStart before running them if we encounter certain errors. This can be done by throwing an error, but this may not always be what you want to do.
3. Returning results from a cache in onQueryStart instead of actually running the query. This would be a pretty advanced use case, and normally I'm inclined to do caching at a higher level, rather than within the database driver, but you certainly could implement this if we added a way to skip running the query and return fake results instead.

We could support the query modification use case by just relaxing the types. You could have something like:

function getCurrentController(): string {
  // TODO: Use AsyncLocalStorage to get the "controller" for the current request
  return 'unknown'
}

function onQueryStart(_query, formattedQuery) {
  // @ts-expect-error formattedQuery.text is marked as readonly
  formattedQuery.text = formattedQuery.text + `/* controller='${getCurrentController()}' */`
}

I'm not confident enough about the caching use case to know what the API should be yet. It's also possible we should add a hook that runs much earlier for this. Ideally, if you call .query(...) on the ConnectionPool, and there's a handler that can respond to your query from the cache, we shouldn't need to get a connection from the connection pool at all. This becomes especially important if your cache is an external service such as Redis/Memcached.

from atdatabases.

@ForbesLindesay thanks for the reply! Turns out this situation I was just trying to force being able to do it through @databases instead of just using triggers.. I was against it but with a little bit of tweaks I was able to get triggers to work as needed.

Still happen to work on a PR for this if it's something you're interested in having (so I can give back), let me know what your thoughts are though, don't want to create additional headache if not needed

from atdatabases.