Aliases in select queries can cause fatal errors about fluent-kit

[gwynne]

Fluent performs column aliasing unconditionally as a defense against name collisions in joins - ironically, I was just telling @0xTim the other day about how to get around exactly this issue; in his case, the problem was the combination of long table names and long column names, and using ModelAlias to shorten the table name was sufficient to avoid the issue. I think you're right that Fluent needs to provide some control over this behavior; at the very least users ought to be able to specify column aliasing as a complement to the ability ModelAlias provides for tables (although admittedly the problem ModelAlias was originally intended to solve has to do with name collisions when joining a table to itself 😅).

I'm not sure of a solution that would solve this "automatically" without requiring support from individual drivers at the very least - this is how we apply SHA-1 hashing for constraint names in MySQL. I hesitate to apply a similar solution elsewhere, however; the hashing just ends up yielding confusing names and requiring even uglier workarounds in the database schema API.

[rausnitz]

It looks like ModelAlias may only work in the context of a join.

final class AliasExample: ModelAlias {
    static let name = "alias_examples"
    let model = DemoUser()
}

The code compiles with that class, but not when I add:

_ = try await AliasExample.query(on: app.db).all()

Build errors:

error: extraneous argument label 'on:' in call
    _ = try await AliasExample.query(on: app.db).all()
                                    ^~~~~
                                     
error: instance member 'query' cannot be used on type 'AliasExample'; did you mean to use a value of this type instead?
    _ = try await AliasExample.query(on: app.db).all()
                  ^~~~~~~~~~~~
error: dynamic key path member lookup cannot refer to static method 'query(on:)'
    _ = try await AliasExample.query(on: app.db).all()

[rausnitz]

Another thing to consider is adding helpful details to this message:

fatalError("Cannot access field before it is initialized or fetched: \(self.key)")

One idea would be to compute the alias string length and add something like this to the fatal error message: "Check if your database's name length limit is smaller than n characters, which is the number of characters used by this field's alias."

[gwynne]

@rausnitz Ooof, you're right, ModelAlias can't be used that way yet. I'll add some APIs for it as soon as I get a chance 😰. My apologies; I haven't actually looked at the darn thing since I overhauled it some time ago (and tbh I don't think I've ever actually used it myself; never had a need in a case where I wasn't already working at the SQLKit layer anyway).

In the meantime, what you can do in such a case is drop down to SQLKit - for the equivalent of the simple .all() query, it would be:

// With aliasing:
let demoUsers = try await (app.db as! any SQLDatabase).select()
    .columns(SQLColumn(SQLLiteral.all, table: SQLIdentifier("alias_examples")))
    .from(SQLAlias(SQLQualifiedTable(DemoUser.schema, space: DemoUser.space), as: SQLIdentifier("alias_examples"))
    .all(decoding: DemoUser.self)

// Without aliasing:
let demoUsers = try await (app.db as! any SQLDatabase).select()
    .columns(SQLLiteral.all)
    .from(SQLQualifiedTable(DemoUser.schema, space: DemoUser.space))
    .all(decoding: DemoUser.self)

(Of course, in SQLKit you don't need the aliasing for a query like this, and the .all(decoding:) method will correctly map the plain column names onto Fluent models.)

[gwynne]

Another thing to consider is adding helpful details to this message:
...

The problem is that that message can come up for several other issues that have nothing to do with aliasing. It's extremely unfortunate that Tanner built this to fatalError() for these cases, but we didn't have effectful accessors (e.g. accessors that can throw) in the language at the time and changing it to be throwing now would horribly break every single Fluent user's code 😞.

FWIW, hitting a length limit on non-key identifiers as you have is not a particularly common problem in our experience - it's only come up two or three times ever, and we don't have a good solution. It's one of the many, many, many, many, many, and did I mention MANY!!! 😅 design flaws in Fluent that we plan to resolve with Fluent 5.

[rausnitz]

@rausnitz Ooof, you're right, ModelAlias can't be used that way yet. I'll add some APIs for it as soon as I get a chance 😰.

If you know what you have in mind, I'd be willing to work on the implementation.

[gwynne]

@rausnitz There are two possible approaches I was thinking of, which aren't mutually exclusive, although one is certainly easier than the other:

1. The easy one - modify QueryBuilder.run() to recognize when self.models.count == 1 && self.models[0] is Model.Type (e.g. there are no joins and the model to return is the same one being queried, overwhelmingly the most common case for Fluent), and in that case, modify its invocation of addFields() to generate model.keys.map { .custom(.sql(SQLIdentifier($0))) } (instead of the usual .extendedPath()), and also avoid invoking the .qualifiedSchema() modifier on output. These changes will trick the SQLQueryConverter and the database output logic into using unqualified column names.

2. The harder one - Define a type something like QueryablePropertyAlias - probably an evolution of the existing AliasedField helper - which does for items conforming to QueryableProperty what ModelAlias does for Schemas . The main problem with this is that ModelAlias works by exploiting the preexisting static alias property of Schema; there is no similar trick to exploit for individual fields. Instead, it would most probably have to work by providing a whole new set of .field() overloads on QueryBuilder which accept only the new alias type and generate fields of the form .custom(.sql(SQLAlias(SQLColumn(SQLIdentifier(property.path[0]), table: SQLQualifiedTable(SQLIdentifier(Property.Model.schemaOrAlias), space: SQLIdentifier(Property.Model.spaceIfNotAliased))), as: SQLIdentifier(self.aliasedName)))) 😰. This would of course be a rotten hack, misusing some of Fluent's existing pieces and definitely not supporting MongoDB at all, but it's all I can think of that wouldn't break public API per se.