planetscale/database-js

Suggestion: Allow setting the `cast` option in `execute`

cdcarson opened this issue ยท 3 comments

tldr;

const result = connOrTx.execute(query, values, { as: 'object',  cast: myOneOffCastFn })

Reasoning

AFAICT you have to pass cast to connect or the Client factory. This is inconvenient when the code that's calling execute isn't the code that created the connection. For example I might have this...

type UserBalance = {
  userId: string; // but stored as INT64
  balance: bigint; // yeah, my app's users are super wealthy
}
const selectUserBalance = async (
  input: { where: Sql, order: Sql, limit: Sql }, 
  db: Transaction | Connection
): Promise<UserBalance[]>  {...}

As it is now selectUserBalance has to re-cast the rows after getting them from the library. If you don't know (or, in my case, never can remember) what the connection's cast is doing, this is hard to get right. Am I casting to BigInt?, etc.

I think it'd be nicer to allow the connection's cast to be overridden or extended as needed. In many (if not all) cases cast is the concern of the model "at hand," not the connection. In addition to BigInt as in the example above, there are at least a couple other cases where one might want to cast on a per-model basis:

  • boolean. The library (rightly) does not try to cast this on its own from say INT8. Even if it did, I'm not sure it's possible to CAST to that in MySQL from an aggregate query. I think the common case, though, is that it should end up as a boolean.
  • Cases where one might want to manipulate the raw data, e.g. turning a float into Decimal or massaging JSON.

How it'd work

  • Folks could still pass a custom "global" cast to connect. This or the library's default cast would be used if execute is called without cast set.
  • It execute is called with cast, the library would use that function rather than the custom or default one.

In userland...

Defining a "global" cast function...

const myGlobalCast = (field: Field, value: string | null) => {
  if (field.type === 'DATETIME' && typeof value === 'string') {
    return new Date(value);
  }
  // etc
  return cast(field, value);
}
const myGetConnection = (url: string): Connection => {
  return connect({
    cast: myGlobalCast,
    url
  });
}

...which can be overridden as need be...

type UserBalance = {
  userId: string; // but stored as INT64
  balance: bigint; // yeah, my app's users are super wealthy
};
const selectUserBalance = async (
  input: { where: Sql; order: Sql; limit: Sql },
  db: Transaction | Connection
): Promise<UserBalance[]> => {
  const { sql: query, values } = sql`some fancy sql here`;
  const cast = (field: Field, value: string | null) => {
    if (field.name === 'balance') {
      return BigInt(value);
    }
    return myGlobalCast(field, value); // or just use the library's default cast
  };
  const result = await db.execute(query, values, { as: 'object', cast });
  return result.rows as UserBalance[];
};

Alternatives Considered

  • Currently I'm short-circuiting the connection's cast with (field: Field, value: string | null) => value, then recasting the fetched rows knowing that I only have to deal with string|null values. This isn't as terrible as it sounds, since it's using cast under the hood for most things, but it's not great.
  • Rename columns to indicate type. Ack. Nope.
  • Maybe I'm missing something?

Hey @cdcarson, I appreciate you opening up this issue!

I had a question with your implementation and suggestion. What is stopping you from using the field.table and the field.name field that is available on the Field type in order to have that special handling at the global connection-level? This way, you don't have to remember to always pass this custom cast function on a per-execution basis and know that for that specific table and column, it will always be casted correctly. Is that still limiting?

Hey @iheanyi, thanks for getting back.

What is stopping you from using the field.table and the field.name field that is available on the Field type in order to have that special handling at the global connection-level?

Consider this query...

SELECT 
  User.id AS id, 
  UserAccount.email IS NOT NULL AS isRegistered
FROM User 
LEFT JOIN UserAccount on UserAccount.userId = User.id
LIMIT 1

It results in these fields from PlanetScale...

[
  {
    "name": "id",
    "type": "UINT64",
    "table": "User",
    "orgTable": "User",
    "database": "<MY DATABASE NAME>",
    "orgName": "id",
    "columnLength": 20,
    "charset": 63,
    "flags": 32801
  },
  {
    "name": "isRegistered",
    "type": "INT64",
    "columnLength": 1,
    "charset": 63,
    "flags": 32897
  }
]

Note the isRegistered field doesn't have table. So we can't rely on table name + field name to do the mapping you suggest.

@cdcarson Sorry for the delay! I made one small tweak to your branch and merged it in, thanks for contributing! ๐Ÿ™๐Ÿพ