drift icon indicating copy to clipboard operation
drift copied to clipboard

Problems with Postgres column types / compatibility in Table definitions

Open tgrushka opened this issue 2 years ago • 1 comments

Describe the bug

boolean() does not work at all in Postgres as the generated CHECK constraint is not needed and throws a cast error.

So one must do one of these:

  BoolColumn get isNonNullableBoolean => boolean().customConstraint('NOT NULL')();
  BoolColumn get isNullableBoolean =>
      boolean().customConstraint('').nullable()();

dateTime() does not work. One must use PgDateTime as follows (but this isn't documented):

const currentTimestamp = CustomExpression<PgDateTime>('CURRENT_TIMESTAMP',
    precedence: Precedence.primary);
...
  TimestampColumn get created =>
      customType(PgTypes.timestampNoTimezone).withDefault(currentTimestamp)();
  TimestampColumn get updated =>
      customType(PgTypes.timestampNoTimezone).withDefault(currentTimestamp)();

I tried to make some idiomatic shortcuts, but they don't work:

abstract class BaseTable extends Table {
  UuidColumn get id => customType(PgTypes.uuid).withDefault(genRandomUuid())();
  @override
  Set<Column> get primaryKey => {id};

  /// Usages fail with: The referenced element could not be analyzed due to a bug in drift, e.g.:
  ///   JsonColumn get address => jsonb().nullable()();
  ColumnBuilder<Object> jsonb() => customType(PgTypes.jsonb);

  @override
  /// Usages fail with: The referenced element could not be analyzed due to a bug in drift.
  /// What if I want a nullable boolean column?
  ColumnBuilder<bool> boolean() => super.boolean().customConstraint('NOT NULL');
  ColumnBuilder<bool> booleanNullable() => super.boolean().customConstraint('NULL').nullable();

  @override
  ColumnBuilder<DateTime> dateTime() => throw UnimplementedError(
      'Use customType(PgTypes.timestampNoTimezone) instead in Postgres!');

  /// Usages fail with: The referenced element could not be analyzed due to a bug in drift, e.g.:
  ///  TimestampColumn get created =>
  ///    timestamp().withDefault(currentTimestamp)();
  ColumnBuilder<PgDateTime> timestamp() =>
      customType(PgTypes.timestampNoTimezone);
}

mixin Timestamps on BaseTable {
  TimestampColumn get created => timestamp().withDefault(currentTimestamp)();
  TimestampColumn get updated => timestamp().withDefault(currentTimestamp)();
}

Mixin columns work:

mixin Timestamps on BaseTable {
  TimestampColumn get created =>
      customType(PgTypes.timestampNoTimezone).withDefault(currentTimestamp)();
  TimestampColumn get updated =>
      customType(PgTypes.timestampNoTimezone).withDefault(currentTimestamp)();
}

But not when using the aliases:

mixin Timestamps on BaseTable {
  TimestampColumn get created =>
      timestamp().withDefault(currentTimestamp)();
  TimestampColumn get updated =>
      timestamp().withDefault(currentTimestamp)();
}

And not in the main table def either:


class Members extends BaseTable with Timestamps {
  // FAILS:
  JsonColumn get address1 => jsonb().nullable()();
  // SUCCEEDS:
  JsonColumn get address2 => customType(PgTypes.jsonb).nullable()();
  // FAILS:
  BoolColumn get isEligibleToVote => boolean()();
  // SUCCEEDS:
  BoolColumn get isEligibleToServe =>
      boolean().customConstraint('NOT NULL')();

  // Has timestamp and id columns from BaseTable and Timestamps mixin.
}

** Expected Behavior **

I expect that, since there are no compile-time errors and the aliased functions are essentially syntactic sugar so I don't have to use the complex incantations on every table for these common Postgres types (can these just be added to drift_postgres?), they should just resolve properly in the generator.

AND... would be nice if Table defs could just be interchangeable / compatible with SQLite <-> Postgres so the table defs could be shared between client and server.

tgrushka avatar Dec 21 '23 20:12 tgrushka

Thanks for reporting all these issues! Having interchangeable table definitions is definitely the goal here.

boolean() does not work at all in Postgres as the generated CHECK constraint is not needed and throws a cast error.

You need to enable the dialect generation option for the dialects you plan on using, by default drift will only use sqlite3. It's horrible that this isn't mentioned on the postgres page, I'll fix that.

dateTime() does not work. One must use PgDateTime as follows (but this isn't documented):

What's going wrong with dateTime()? We seem to have tests for that, but perhaps they are missing something?

I tried to make some idiomatic shortcuts, but they don't work:

Yeah unfortunately those all can't work since we essentially have to look at the syntax of the table to figure out its schema, these aren't proper macros that could run at compile time.

simolus3 avatar Dec 30 '23 23:12 simolus3