Column Types

[MarkHerhold]

I've been hitting a lot of Missing type: X column errors (where X is uuid, int4, text, and custom enum). It looks like I could just add each DB type until I realized we would need to account for custom enumerations in PostgreSQL and provide those or a parser callback to rds-data. Also, there are an insane number of DB types that we would have to try to assume how to handle, which is never good.

I think that defining each column type for multiple DBs is probably just a loosing battle. Switch statement in question: https://github.com/cbschuld/rds-data/blob/49d6f2fa2f18e7321d081d0f8434dd6785be133d/src/RDSData.ts#L184-L213

As an alternative, we could just return the anticipated value and not rely on column.typeName. Developers will know what type their data should be.

class ColumnValue {
  public field: Field; // dev can always pull original Field object

  constructor(field: Field) {
    this.field = field;
  }

  get isNull(): boolean {
    return !!this.field.isNull;
  }

  get date(): Date | null {
    if (this.isNull) return null;
    return new Date(this.field.stringValue!);
  }

  get string(): string | null {
    if (this.isNull) return null;
    return this.field.stringValue;
  }

  get number(): number | null {
    if (this.isNull) return null;
    return this.field.longValue;
  }

  // same idea for buffer, boolean, etc
}

Pros:

• getters reduce unnecessary computation
• great future compatibility for unanticipated types, custom DB enums/types, no more "error by default"
• less boilerplate
• library doesn't try to be smarter than the developer
• library doesn't assume that binary represented as a string should be Base64
• library doesn't return undefined when true value is null (I believe that's what is currently happening)

Cons:

• slight complexity increase with class
• slight interface change (technically breaking)
• proposed solution is still not as clean as a hydrator function

What do you think @cbschuld ?

[cbschuld]

@MarkHerhold I like this proposal for sure - it is breaking but I do not think there is heavy use yet so I am fine to take the heat for a breaking change now

[cbschuld]

Mark, should we keep the types relative to the RDS library (such as RDSColumnValue) or do you feel ColumnValue is better (I like plain ColumnValue better IMO).

[MarkHerhold]

Since it's not an AWS implementation, I'd stick with ColumnValue but I'm always happy to hear other opinions.

I can work on a PR for this as well. I have a number of changes going on concurrently as I try to get this library to work and am making issues as I investigate. 👍

[cbschuld]

@MarkHerhold - I don't disagree. Also, it maybe a best-foot-forward time to rename some of the other types as well.

[MarkHerhold]

Just adding notes: I need enums to work with this lib as well, meaning we can't hardcode types (unless we provide overrides).

test('select an enum', async () => {
  const rds = setupRDSDatabase().getInstance();
  await rds.query(
  `DO $$ BEGIN
    CREATE TYPE enum_abc AS ENUM ('a', 'b', 'c');
  EXCEPTION
    WHEN duplicate_object THEN null;
  END $$;`);
  const results = await rds.query(`select 'a'::enum_abc as value, pg_typeof('a'::enum_abc) as type`);

  expect(results.data[0].value.string).toEqual('a');
  expect(results.data[0].type.string).toEqual('enum_abc');
});

[spolom]

I ran into this too, while trying out this great looking lib out for the first time today. E.g. PostgreSQL uses timestamptz for select now() and when I casted it to a char it became a text.