Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Table primary composite key from Foreign Keys fails in 2.20 #3173

Open
velten-dev opened this issue Aug 23, 2024 · 7 comments
Open

Table primary composite key from Foreign Keys fails in 2.20 #3173

velten-dev opened this issue Aug 23, 2024 · 7 comments
Labels
area-manager Related to the manager, exposing an easier to use interface bug Something isn't working package-drift_dev Affects the drift_dev package

Comments

@velten-dev
Copy link

When I upgraded from Drift 2.19 to 2.20, I get build errors stemming from code generate for a table which has a primaryKey attribute formed from foreign keys. I have a financial tracker, so I implemented transfers by making them a table that holds a reference to two transaction keys and has a primary key defined by the composite of the two foreign keys.

class TransactionTransfers extends Table {
  @ReferenceName('sourceTxUuid')
  TextColumn get sourceUuid =>
      text().references(Transactions, #uuid, onDelete: KeyAction.cascade)();
  @ReferenceName('destinationTxUuid')
  TextColumn get destinationUuid =>
      text().references(Transactions, #uuid, onDelete: KeyAction.cascade)();

  @override
  Set<Column<Object>>? get primaryKey => {sourceUuid, destinationUuid};
}

class Transactions extends Table {
  TextColumn get uuid => text()();
  DateTimeColumn get date => dateTime()();
  IntColumn get amount => integer()();
  // etc, etc

  @override
  Set<Column>? get primaryKey => {uuid};
}

When I try to build/run I get the following errors:

Got dependencies!
11 packages have newer versions incompatible with dependency constraints.
Try `flutter pub outdated` for more information.

ERROR: lib/database/reckoner_db.g.dart:12599:16: Error: The getter 'sourceUuid' isn't defined for the class 'TransactionTransfer'.
ERROR:  - 'TransactionTransfer' is from 'package:reckoner/model/transaction.dart' ('lib/model/transaction.dart').
ERROR: Try correcting the name to the name of an existing getter, or defining a getter or field named 'sourceUuid'.
ERROR:     if ($_item.sourceUuid == null) return null;
ERROR:                ^^^^^^^^^^
ERROR: lib/database/reckoner_db.g.dart:12601:38: Error: The getter 'sourceUuid' isn't defined for the class 'TransactionTransfer'.
ERROR:  - 'TransactionTransfer' is from 'package:reckoner/model/transaction.dart' ('lib/model/transaction.dart').
ERROR: Try correcting the name to the name of an existing getter, or defining a getter or field named 'sourceUuid'.
ERROR:         .filter((f) => f.uuid($_item.sourceUuid!));
ERROR:                                      ^^^^^^^^^^
ERROR: lib/database/reckoner_db.g.dart:12613:16: Error: The getter 'destinationUuid' isn't defined for the class 'TransactionTransfer'.
ERROR:  - 'TransactionTransfer' is from 'package:reckoner/model/transaction.dart' ('lib/model/transaction.dart').
ERROR: Try correcting the name to the name of an existing getter, or defining a getter or field named 'destinationUuid'.
ERROR:     if ($_item.destinationUuid == null) return null;
ERROR:                ^^^^^^^^^^^^^^^
ERROR: lib/database/reckoner_db.g.dart:12615:38: Error: The getter 'destinationUuid' isn't defined for the class 'TransactionTransfer'.
ERROR:  - 'TransactionTransfer' is from 'package:reckoner/model/transaction.dart' ('lib/model/transaction.dart').
ERROR: Try correcting the name to the name of an existing getter, or defining a getter or field named 'destinationUuid'.
ERROR:         .filter((f) => f.uuid($_item.destinationUuid!));
ERROR:                                      ^^^^^^^^^^^^^^^
ERROR: Target kernel_snapshot_program failed: Exception
Building Linux application...                                           
Build process failed
make: *** [makefile:47: linux] Error 1

If I change the table definition and remove the foreign key references, everything compiles correctly and appears to be working based on my demo dataset.

class TransactionTransfers extends Table {
  @ReferenceName('sourceTxUuid')
  TextColumn get sourceUuid => text()();
  @ReferenceName('destinationTxUuid')
  TextColumn get destinationUuid => text()();

  @override
  Set<Column<Object>>? get primaryKey => {sourceUuid, destinationUuid};
}

What changed to make this setup fail?

@simolus3
Copy link
Owner

simolus3 commented Aug 24, 2024

Thanks for the report! It's possible that this is a regression in 2.20.0 since that release contains new features for resolving references more easily, but I can't reproduce it with the tables you've posted alone. This file generates valid code for me:

import 'package:drift/drift.dart';

part 'repro.g.dart';

class TransactionTransfers extends Table {
  @ReferenceName('sourceTxUuid')
  TextColumn get sourceUuid =>
      text().references(Transactions, #uuid, onDelete: KeyAction.cascade)();
  @ReferenceName('destinationTxUuid')
  TextColumn get destinationUuid =>
      text().references(Transactions, #uuid, onDelete: KeyAction.cascade)();

  @override
  Set<Column<Object>>? get primaryKey => {sourceUuid, destinationUuid};
}

class Transactions extends Table {
  TextColumn get uuid => text()();
  DateTimeColumn get date => dateTime()();
  IntColumn get amount => integer()();
  // etc, etc

  @override
  Set<Column>? get primaryKey => {uuid};
}

@DriftDatabase(tables: [TransactionTransfers, Transactions])
class Database extends _$Database {
  Database(super.e);

  @override
  int get schemaVersion => 1;
}

Are there any other references towards TransactionTransfers that might be causing this?

@velten-dev
Copy link
Author

velten-dev commented Aug 24, 2024

I'll work on trying to reproduce this with a smaller set of data. It will be a little bit before I have the time to experiment with and create a smaller reproducer.

I encountered this issue on both Mac and Linux with Drift 2.20 for my app Reckoner. I don't have any data type referencing TransferTransactions, but there is a lot of references to various tables from other tables. You can look at the source to see my table setup for other red flags. I think I'm doing something a bit unusual where I use inheritance to keep the code DRY and make common functions for similar data types. For example, a table class called TrackedTable (not a database table) which creates uuid text field and overrides the primary key to be the uuid. I used this before you released managers and I've noted that managers don't seem to be generated at all/correctly for my database when I tried to use them.

I'll probably just create a copy with the Reckoner Drift setup and remove things until I don't hit the issue anymore. On the upside, I can continue to build using 2.19 and I already setup my repo to fix my dependencies.

@simolus3
Copy link
Owner

Don't worry about finding a minimal case to reproduce it, I can also look at the entire repository and try to figure it out from there. Do you have a pushed commit somewhere that's pointing towards the state this was in when you ran into the error? Or can I just try to upgrade drift on main to hit this?

@velten-dev
Copy link
Author

It should be just as easy as checking out main and upgrading drift.

If you're cool with running my repo, you would just need to do make upgrade and then make linux. I also have an install script to make sure dependencies are installed for debian based distros (install.sh). Other than the dependencies for SQLCipher, I depend on libjson for secret management and libayatana-appindicator3 for a tray manager. You can build it without the tray manager by removing code depending on tray_manager. It wouldn't be feasible to remove the secret management code without significantly more work.

Shouldn't need to install anything if testing on a mac. I just merged the v2.0.0 changes which add support for mac and iOS.

@simolus3
Copy link
Owner

Hm this is a tricky one. We currently expect the data classes to have getters for columns involved in references (because the reference functionality of the manager operates on resolved data classes, which generally makes things more intuitive for users).

I've fixed it by just adding the missing getters here:

diff --git a/lib/model/transaction.dart b/lib/model/transaction.dart
index 15df54d..085e72a 100644
--- a/lib/model/transaction.dart
+++ b/lib/model/transaction.dart
@@ -148,6 +148,9 @@ class TransactionTransfer extends Common<TransactionTransfer>
         destinationUuid: Value(destination.uuid),
       );
 
+  String get sourceUuid => source.uuid;
+  String get destinationUuid => destination.uuid;
+
   @override
   Object get key => (source.uuid, destination.uuid);

@simolus3 simolus3 added bug Something isn't working area-manager Related to the manager, exposing an easier to use interface package-drift_dev Affects the drift_dev package labels Aug 25, 2024
@dickermoshe
Copy link
Collaborator

@simolus3 Is this my fault?

@simolus3
Copy link
Owner

Absolutely not, I was the one adding manager support to custom row classes. I don't think there's a general solution for this, but perhaps we can skip generating references code depending on whether the fields are present on the custom row class. I'll take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-manager Related to the manager, exposing an easier to use interface bug Something isn't working package-drift_dev Affects the drift_dev package
Projects
None yet
Development

No branches or pull requests

3 participants