Uh oh!
There was an error while loading. Please reload this page.
Update enum quoting and default schema use - #605
Conversation
0e37395 to
8816c0fCompare7a4a035 to
f3f7c9fComparef3f7c9f to
b6f2c7cComparec851233 to
b166e71Compareb166e71 to
71a6464CompareDoes this request fix #623? If not, please fix it in a different PR, not here. This would make it easier to track changes. |
austindrenski
commented
Aug 31, 2018
@YohDeadfall I mistakenly thought there would be some overlap in the issues, but they are indeed distinct. None of the changes included here are intended to address #623 (other than improving enum support, generally). |
Uh oh!
There was an error while loading. Please reload this page.
austindrenski
commented
Sep 1, 2018
Updated to |
cbc1f47 to
b93e1eeCompareaustindrenski
commented
Sep 1, 2018
Rebased and updated to reflect #621. |
roji
commented
Sep 4, 2018
@austindrenski I'll give this a full review, but can you please make clearer exactly what this PR covers? Is the description above still valid or has the scope of this PR changed? Regardless, one thing that immediately caught my eye:
This doesn't seem like the correct EF Core behavior. Executing the following:
Results in two tables being created under |
BTW does this depend on npgsql/npgsql#2121, whose PR npgsql/npgsql#2123 still seems to be WIP? |
austindrenski
commented
Sep 4, 2018
Sure, I'll clean that up today. This PR grew and shifted a bit since I opened it, so the current description is a bit of lazy editing on my part.
Early on for EF Core, I thought I read some docs that discussed order being relevant in configuration, but I can't seem to find anything like that now. (Could have been a third-party blog, TBH.) What you describe makes more sense (and matches the internals too). I can go back through and move the default schema detection later in the process.
No, not anymore. An earlier draft was, but I reworked this one to use the simple "split on the first dot" approach while I work through npgsql/npgsql#2121 (which has turned out to be a bit more confusing than I first imagined). |
04dd4d3 to
46298b7Compare46298b7 to
2a6bea2Compareaustindrenski
commented
Sep 29, 2018
@roji The description is up to date, and I've rebased for the current I'd like to see this make it into @YohDeadfall Could you also give this another review? |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
c758832 to
5f8e109Compareaustindrenski
commented
Oct 1, 2018
@YohDeadfall Appreciate the review—updated. |
- Properly quote schema and type name - Respect default schema annotation - Cache enum values and simplify non-null literal generation
5f8e109 to
1454851Compareaustindrenski
commented
Oct 24, 2018
I'd like to merge this and #626 tomorrow at 1:00 PM (UTC). If anyone wants more time for another round of review, just let me know. |
Changes
.ForNpgsqlHasEnum(...), then use the default schema annotation.StoreType).Fixes: #554
Fixes: #593