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

Support CAST for special dart types and MAPPED BY in queries #2235

Closed
North101 opened this issue Dec 26, 2022 · 13 comments
Closed

Support CAST for special dart types and MAPPED BY in queries #2235

North101 opened this issue Dec 26, 2022 · 13 comments
Labels
enhancement New feature or request

Comments

@North101
Copy link
Contributor

It would be nice to support CAST for special dart types (e.g. ENUMNAME, BOOLEAN, DATETIME) and MAPPED BY in queries.

SELECT
  table.*,
  '{}' MAPPED BY `const JsonTypeConverter()` AS value,
  CAST('new' AS ENUMNAME(State)) AS state
FROM table
WHERE something

(Not the best example)

@North101 North101 added the enhancement New feature or request label Dec 26, 2022
@simolus3
Copy link
Owner

simolus3 commented Jan 5, 2023

MAPPED BY is supported in e562cb9.

@North101
Copy link
Contributor Author

North101 commented Jan 6, 2023

I'm getting an error here:
e562cb9#diff-70e1bc0c02a904278b060b1f37b99eeea524ab64c23bccd61bc2bf02dacb5370R152

I also tried to using it in a view and it didn't work there (though maybe thats more complicated)

Null check operator used on a null value
package:drift_dev/src/analysis/resolver/queries/query_analyzer.dart 153:29  QueryAnalyzer._resolveDartTokens
package:drift_dev/src/analysis/resolver/queries/query_analyzer.dart 98:11   QueryAnalyzer.analyze
package:drift_dev/src/analysis/resolver/file_analysis.dart 104:30           FileAnalyzer.runAnalysisOn
package:drift_dev/src/analysis/driver/driver.dart 225:22                    DriftAnalysisDriver.fullyAnalyze
package:drift_dev/src/backends/build/drift_builder.dart 158:20              _DriftBuildRun._analyze
package:drift_dev/src/backends/build/drift_builder.dart 296:11              _DriftBuildRun._generateMonolithic
package:drift_dev/src/backends/build/drift_builder.dart 150:7               _DriftBuildRun.run
package:drift_dev/src/backends/build/drift_builder.dart 100:5               DriftBuilder.build

@North101
Copy link
Contributor Author

North101 commented Jan 6, 2023

Just tried with bc325dd and its working!

@North101
Copy link
Contributor Author

North101 commented Jan 6, 2023

Ah, spoke too soon. The converter isn't being generated (I'm using EnumNameConverter if that makes a difference).

Generated View:

class RotationView extends ViewInfo<RotationView, RotationViewData>
    implements HasResultSet {
  final String? _alias;
  @override
  final _$Database attachedDatabase;
  RotationView(this.attachedDatabase, [this._alias]);
  @override
  List<GeneratedColumn> get $columns =>
      [code, rotationCode, formatCode, name, dateStart, type];
  @override
  String get aliasedName => _alias ?? entityName;
  @override
  String get entityName => 'rotation_view';
  @override
  String get createViewStmt =>
      'CREATE VIEW rotation_view (code,rotation_code,format_code,name,date_start,type) AS SELECT rotation.code, rotation.rotation_code, rotation.format_code, rotation.name, rotation.date_start, rotation.type FROM (SELECT format.code || \'@current\' AS code, rotation.code AS rotation_code, format.code AS format_code, format.name || \' Current (\' || COALESCE(rotation.name, \'None\') || \')\' AS name, rotation.date_start, \'current\' AS type FROM format LEFT JOIN (SELECT *, MAX(date_start) FROM rotation WHERE DATE(date_start) <= DATE(\'NOW\') GROUP BY rotation.format_code) AS rotation ON rotation.format_code = format.code UNION ALL SELECT format.code || \'@latest\' AS code, rotation.code AS rotation_code, format.code AS format_code, format.name || \' Latest (\' || COALESCE(rotation.name, \'None\') || \')\' AS name, rotation.date_start, \'latest\' AS type FROM format LEFT JOIN (SELECT *, MAX(date_start) FROM rotation GROUP BY rotation.format_code) AS rotation ON rotation.format_code = format.code UNION ALL SELECT rotation.code, rotation.code AS rotation_code, rotation.format_code, rotation.name, rotation.date_start, NULL AS type FROM rotation) AS rotation INNER JOIN format ON format.code = rotation.format_code ORDER BY format.id, rotation.type DESC NULLS LAST, rotation.date_start DESC';
  @override
  RotationView get asDslTable => this;
  @override
  RotationViewData map(Map<String, dynamic> data, {String? tablePrefix}) {
    final effectivePrefix = tablePrefix != null ? '$tablePrefix.' : '';
    return RotationViewData(
      code: attachedDatabase.typeMapping
          .read(DriftSqlType.string, data['${effectivePrefix}code'])!,
      rotationCode: attachedDatabase.typeMapping
          .read(DriftSqlType.string, data['${effectivePrefix}rotation_code']),
      formatCode: attachedDatabase.typeMapping
          .read(DriftSqlType.string, data['${effectivePrefix}format_code'])!,
      name: attachedDatabase.typeMapping
          .read(DriftSqlType.string, data['${effectivePrefix}name'])!,
      dateStart: attachedDatabase.typeMapping
          .read(DriftSqlType.dateTime, data['${effectivePrefix}date_start']),
      // RotationView.$convertertypen isn't being generated
      type: RotationView.$convertertypen.fromSql(attachedDatabase.typeMapping
          .read(DriftSqlType.string, data['${effectivePrefix}type'])),
    );
  }

  late final GeneratedColumn<String> code = GeneratedColumn<String>(
      'code', aliasedName, false,
      type: DriftSqlType.string);
  late final GeneratedColumn<String> rotationCode = GeneratedColumn<String>(
      'rotation_code', aliasedName, true,
      type: DriftSqlType.string);
  late final GeneratedColumn<String> formatCode = GeneratedColumn<String>(
      'format_code', aliasedName, false,
      type: DriftSqlType.string);
  late final GeneratedColumn<String> name = GeneratedColumn<String>(
      'name', aliasedName, false,
      type: DriftSqlType.string);
  late final GeneratedColumn<DateTime> dateStart = GeneratedColumn<DateTime>(
      'date_start', aliasedName, true,
      type: DriftSqlType.dateTime);
  late final GeneratedColumnWithTypeConverter<RotationType?, String> type =
      GeneratedColumn<String>('type', aliasedName, true,
              type: DriftSqlType.string)
          .withConverter<RotationType?>(RotationView.$convertertypen);
  @override
  RotationView createAlias(String alias) {
    return RotationView(attachedDatabase, alias);
  }

  @override
  Query? get query => null;
  @override
  Set<String> get readTables => const {'format', 'rotation'};
}

RotationView.$convertertypen isn't being generated

@simolus3
Copy link
Owner

simolus3 commented Jan 6, 2023

As of a6bede2, type converters for views should be generated.

I've also added support for casts to drift-specific SQL types.

@simolus3 simolus3 closed this as completed Jan 6, 2023
@North101
Copy link
Contributor Author

North101 commented Jan 6, 2023

Works mostly for me. Although CAST seems like it makes the column non-null (MAPPED BY is working as expected with NULL columns).

I noticed this because something odd happened when I changed MAPPED BY const EnumNameConverter<MyEnum>(MyEnum.values) to CAST(value AS ENUMNAME(MyEnum)). Rows where the value was null were no longer appearing.

@simolus3
Copy link
Owner

simolus3 commented Jan 6, 2023

Although CAST seems like it makes the column non-null

Yikes, that should be fixed in 4b83810.

@North101
Copy link
Contributor Author

North101 commented Jan 7, 2023

@simolus3 I'm still getting an error on 4b83810:

flutter: Null check operator used on a null value
flutter: #0      RotationView.map
package:netrunner_deckbuilder/db/database.g.dart:1263
#1      TableInfoUtils.mapFromRow
package:drift/…/schema/table_info.dart:123
#2      TableInfoUtils.mapFromRow.<anonymous closure>
package:drift/…/schema/table_info.dart:122
#3      _AsyncMappedSelectable._mapResults
package:drift/…/statements/query.dart:311
<asynchronous suspension>
#4      Stream.asyncMap.<anonymous closure>.add (dart:async/stream.dart:793:7)
<asynchronous suspension>

Also, using CAST(NULL AS EnumName(MyEnum)) doesnt generate a nullable TypeConverter.

Never mind, that seemed to be a cache bug (I've had this a couple times recently). I was definitely on 4b8310 when I tested it previously, but its now consistently generating the correct code.

@kevinetore
Copy link

kevinetore commented Dec 29, 2023

@simolus3 @North101 Hopefully one of you could shed some light on my situation. My case strongly feels like the issue mentioned in your earlier comments.

I have a drift file which creates a view:

CREATE VIEW my_view AS
    SELECT
        CAST(id AS INTEGER) AS id,
        location.createdAt,
        CAST(location.country AS ENUMNAME(COUNTRY)) AS country,
        ...
    FROM x
    INNER JOIN ...
    LEFT JOIN ...
    WHERE ...

This generates a ViewData class:

class ViewData extends DataClass {
  final int id;
  final String? createdAt;
  final String country;
  const ViewData()

What I'm trying to achieve is that both id and country would be nullable inside the ViewData class, their corresponding Table columns are nullable. However, it seems that the CAST operation makes it non-nullable. I've also tried some experiments with wrapping the CAST inside a NULLIF condition.

Drift version:

drift: ^2.14.1

@simolus3
Copy link
Owner

That does indeed sound very similar to the previous problem. Nullable columns should never be made non-nullable and we have tests for the previous bug, but the analysis isn't exactly straightforward and I wonder whether the joins could trip it up for instance. Do you have a way to share fairly complete code that reproduces the issue? Then I can take a look at the intermediate analysis changes to see where wrong result is coming from.

@kevinetore
Copy link

@simolus3 Morning. I've made a demo project that reproduces the issue, you can find it here. In the demo project I have:

Enum class
COUNTRY

Drift Table

@UseRowClass(DriftCastModel)
class DriftCast extends Table {
  IntColumn get id => integer().autoIncrement().nullable()();
  TextColumn get country => textEnum<COUNTRY>().nullable()();
}

class DriftCastModel extends Insertable<DriftCastModel> {
  final int? id;
  final COUNTRY? country;

  DriftCastModel({
    this.id,
    this.country,
  });

  ...
}

And a view via tables.drift

CREATE VIEW drift_cast_view AS
    SELECT
        CAST(id AS INTEGER) AS id,
        CAST(country AS ENUMNAME(COUNTRY)) AS country
    FROM DriftCast;

Which generates

class DriftCastViewData extends DataClass {
  final int id;
  final String country;
  const DriftCastViewData({required this.id, required this.country});
  ...
}

@simolus3
Copy link
Owner

After adding the necessary imports to the drift file (which also fixes the warnings emitted by drift_dev), it works and generates nullable fields:

import 'package:drift_cast/features/drift_cast/data/model/drift_cast.dart';
import 'package:drift_cast/core/constants/country.dart';

CREATE VIEW drift_cast_view AS
    SELECT
        CAST(id AS INTEGER) AS id,
        CAST(country AS ENUMNAME(COUNTRY)) AS country
    FROM drift_cast;

You also need to replace DriftCast with drift_cast in the FROM clause because drift uses snake case when converting the name of Dart classes to SQL.

Technically drift could have generated the columns as nullable before, but due to the unresolved table reference it had no idea about their types and essentially guessed their nullability. I think that's fine since the view had an inalid definition, so attempting to create it in the database would have failed as well.

@kevinetore
Copy link

Nice! I've updated the drift file with your suggestions and everything clicked 👍 .

Thanks for your quick responses, amazing library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants