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

simplify table name generation #492

Closed
tanner0101 opened this issue May 24, 2018 · 5 comments
Closed

simplify table name generation #492

tanner0101 opened this issue May 24, 2018 · 5 comments
Labels
enhancement New feature or request
Projects
Milestone

Comments

@tanner0101
Copy link
Member

Currently, Fluent just adds an s to the end of the lowercased type name to generate table names. This has better performance than trying to generate correct plural forms of the words, but is obviously not correct in all cases. It is also not as performant as it could be since it requires lowercasing the type name. It is awkwardly in between being correct sometimes and being performant. I propose two different solutions:

  1. Just use the type name as the table name:

IMO, there's not a huge benefit to trying to transform the type name into a traditional SQL name. All strings in Fluent are escaped anyway. If we're being utilitarian, there's no reason to waste CPU resources generating "prettier" names you shouldn't have to interact with them directly anyway.

This would be a breaking change, but is easily remedied by implementing static let entity: String and would be much more performant.

  1. Do a good job generating correct table names:

This would require quite a bit of work to get right--english is quite complex. Avoiding regex would be key. And ideally we would want to cache the result of the computation so that it doesn't affect performance. Although this is debatable since you could just implement the static let entity: String property if you wanted the extra performance. The key is implementing this is a nice, maintainable way.

Thoughts?

@jdmcd
Copy link
Member

jdmcd commented May 24, 2018

Strongly in favor of option 1. I don’t think the (very marginal) benefits of options 2 outweigh the downfalls.

@anthonycastelli
Copy link
Member

I agree with @mcdappdev. I customize all of my table names manually to fit me. Letting it generate it automatically is helpful, but doesn't add any benefit in my opinion.

@MrMage
Copy link
Contributor

MrMage commented May 24, 2018 via email

@grundoon
Copy link
Member

grundoon commented May 24, 2018

The only winner in (2) is the grammar purist, and even then there is no One True Purity (persons vs people, fish vs fishes). Words whose plural is no change (sheep, deer). Words that have no plural (who, what). Model AttorneyGeneral sounds like fun.

(1) is the sensible, almost surely right, path. But breaking change... is it not possible to reflect an existing entity's name into entity on migration?

I suppose the only other option is to swap the (rather naïve) simple s for something agnostic like DB, which would possibly allay the police while providing no other real advantage.

@tanner0101 tanner0101 added this to the 3.0.0 milestone May 25, 2018
@tanner0101 tanner0101 added the enhancement New feature or request label May 25, 2018
@grosch
Copy link

grosch commented May 26, 2018

This seems strange to me. I like the default of adding the 's' as it's right more often than not, at least in my use cases. The one time it was wrong, I simply added the static let entity to the table and the problem was solved.

ckd added a commit to ckd/documentation that referenced this issue Jul 8, 2018
0xTim added a commit to vapor/docs that referenced this issue Jul 8, 2018
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
No open projects
Vapor 3
  
Awaiting triage
Development

No branches or pull requests

6 participants