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:

rds-data/src/RDSData.ts

Lines 184 to 213 in 49d6f2f

	switch (columns[c].type) {
	case 'BINARY':
	v.buffer = isNull ? undefined : Buffer.from((record[c].blobValue || '').toString());
	v.string = isNull ? undefined : v.buffer?.toString('base64');
	break;
	case 'BIT':
	v.boolean = isNull ? undefined : record[c].booleanValue;
	v.number = v.boolean ? 1 : 0;
	break;
	case 'TIMESTAMP':
	case 'DATETIME':
	case 'DATE':
	v.date = isNull ? undefined : new Date(record[c].stringValue ?? '');
	v.string = isNull ? undefined : record[c].stringValue;
	v.number = v.date ? v.date.getTime() : 0;
	break;
	case 'INT':
	case 'INT UNSIGNED':
	case 'BIGINT':
	case 'BIGINT UNSIGNED':
	v.number = isNull ? undefined : record[c].longValue;
	break;
	case 'TEXT':
	case 'CHAR':
	case 'VARCHAR':
	v.string = isNull ? undefined : record[c].stringValue;
	break;
	default:
	throw new Error(`Missing type: ${columns[c].type}`);
	}

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.