vapor / fluent-kit

Swift ORM (queries, models, and relations) for NoSQL and SQL databases
MIT License
217 stars 116 forks source link

Fatal error "Non-uniform query input" when creating models from a collection. #594

Closed nastynate13 closed 9 months ago

nastynate13 commented 9 months ago

Describe the bug

Fatal error "Non-uniform query input" when creating models from a collection.

To Reproduce

The issue seems to be triggered when:

  1. the model in question has an optional field

and

  1. the models within the collection of models differ in that their optional field is either a) set to a value or b) nil, but not explicitly set to nil in the model's initializer

Steps to reproduce the behavior:

import Fluent

final class Foo: Model {
  static let schema = "foos"
  @ID()
  var id: UUID?

  @OptionalField(key: "maybe_nil")
  var maybeNil: String?

  init() {}

  init(id: UUID? = nil, maybeNil: String?, explicitySetToNil: Bool) {
    self.id = id
    if !explicitySetToNil && maybeNil == nil {
      // maybeNil will not be explicitly set to nil
      return
    }
    // maybeNil will always be explicitly set to the value or nil
    self.maybeNil = maybeNil
  }
}

struct CreateFoo: AsyncMigration {
  func prepare(on database: Database) async throws {
    try await database.schema(Foo.schema)
      .id()
      .field("maybe_nil", .string)
      .create()
  }
  func revert(on database: Database) async throws {
    try await database.schema(Foo.schema).delete()
  }
}

/// creates 2 foos on the db from a Collection<Foo>
///
/// `Foo.maybeNil` is nil on one and is set to value "not nil" on the other
func createFoosFromCollection(on db: Database, explictlySetToNil: Bool) async throws {
  let nilFoo = Foo(maybeNil: nil, explicitySetToNil: explictlySetToNil)
  let nonNilFoo = Foo(maybeNil: "not nil", explicitySetToNil: explictlySetToNil)
  let fooCollection = [nonNilFoo, nilFoo]
  try await fooCollection.create(on: db)
}

in configure(_ app: Application)

to reproduce the crash:

    app.migrations.add(CreateFoo())

    do {
      try await createFoosFromCollection(on: app.db, explictlySetToNil: false)
    } catch {
      debugPrint(error)
    }

debug crash log:

[ DEBUG ] query create foos input=[[id: E88693CF-100F-496B-9019-57F7DA4AA134, maybe_nil: "not nil"], [id: 1B224F66-F7CD-4A93-B7EA-8371FD1ED913]] [database-id: psql] (FluentKit/QueryBuilder.swift:293) FluentSQL/SQLQueryConverter.swift:107: Fatal error: Non-uniform query input: [[id: E88693CF-100F-496B-9019-57F7DA4AA134, maybe_nil: "not nil"], [id: 1B224F66-F7CD-4A93-B7EA-8371FD1ED913]]

to eliminate the crash:

explicit set the the Foo.maybeNil value to nil in the model's initializer (as opposed to leaving it unset)

    app.migrations.add(CreateFoo())

    do {
      try await createFoosFromCollection(on: app.db, explictlySetToNil: true)
    } catch {
      debugPrint(error)
    }

Expected behavior

No error - just as if the models were created one by one. The following code does not produce the error, even when the optional fields are left unset.

/// creates 2 `Foo`s on the db one by one
///
/// `Foo.maybeNil` is nil on one and is set to value "not nil" on the other
func createFoosOneByOne(on db: Database, i: Bool) async throws {
  let nilFoo = Foo(maybeNil: nil, explicitySetToNil: false)
  let nonNilFoo = Foo(maybeNil: "not nil", explicitySetToNil: false)

  try await nilFoo.create(on: db)
  try await nonNilFoo.create(on: db)
}

Environment

package details:

    platforms: [
       .macOS(.v14)
    ],
    dependencies: [
        .package(url: "https://github.com/vapor/vapor.git", from: "4.92.2"),
        .package(url: "https://github.com/vapor/fluent.git", from: "4.9.0"),
        .package(url: "https://github.com/vapor/fluent-postgres-driver.git", from: "2.8.0")
   ]
gwynne commented 9 months ago

Hm... That's a bug alright, and the practical upshot of it is that the uniformity check isn't really sensible unless the input mapping is smart enough to use an explicit default literal... looks like I need to tweak the logic for .create() to explicitly request defaulted values for unset fields. I'll put up a fix for this shortly, thanks for reporting!

nastynate13 commented 9 months ago

Thank you!