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

Enable getting views from schema query (for EF Core 3.0) #233

Closed
5 tasks done
ErikEJ opened this issue Jun 29, 2019 · 40 comments · Fixed by #265
Closed
5 tasks done

Enable getting views from schema query (for EF Core 3.0) #233

ErikEJ opened this issue Jun 29, 2019 · 40 comments · Fixed by #265

Comments

@ErikEJ
Copy link
Owner

ErikEJ commented Jun 29, 2019

If EF Core 3.0, get at concatenated list of:
1: tables sorted by named
2: views sorted by name

  • Implement for SQL Server @ErikEJ
  • Implement for Dacpac @ErikEJ
  • Implement for SQLite @ErikEJ
  • Implement for Postgres
  • Implement for MySQL
@ErikEJ ErikEJ changed the title Enable getting views fro schema query (for EF Core 3.0) Enable getting views from schema query (for EF Core 3.0) Jun 29, 2019
@ErikEJ
Copy link
Owner Author

ErikEJ commented Jun 29, 2019

Parts implemented in 63f7129
@jespersh @roji Maybe you could help?

@jespersh
Copy link
Contributor

Sure thing. Do you have an example Create view statement and its expected code generation?

Probably won't need it, but nice for verification

@ErikEJ
Copy link
Owner Author

ErikEJ commented Jun 30, 2019

It is just for reverse engineering, and this is just the first, inactive baby step.

The new fluent property is .HasNoKey()

@jespersh
Copy link
Contributor

jespersh commented Aug 1, 2019

My part for MySQL is already showing views in the list actually, but it is showing them up as "This table has no primary key and will be ignored", so I am unsure if there's more to be done for me?

@ErikEJ
Copy link
Owner Author

ErikEJ commented Aug 1, 2019

@jespersh great. The message is a UI issue.

@ErikEJ
Copy link
Owner Author

ErikEJ commented Aug 2, 2019

@roji Ping?

@roji
Copy link

roji commented Aug 9, 2019

Sorry for not responding on this.

@ErikEJ this is about having EFCore.PG scaffold views right (like dotnet/efcore#1679)? If so I've opened npgsql/efcore.pg#972 to track and will probably complete this for preview8. Thanks for poking me on it :)

@roji
Copy link

roji commented Aug 9, 2019

So it turns out I'm getting senile, and already did this (npgsql/efcore.pg#878). Is there anything else you need from me here?

@ErikEJ
Copy link
Owner Author

ErikEJ commented Aug 9, 2019

@ErikEJ
Copy link
Owner Author

ErikEJ commented Aug 9, 2019

@jespersh
Copy link
Contributor

jespersh commented Aug 9, 2019

@ErikEJ I can easily pick out the data here: https://github.com/ErikEJ/EFCorePowerTools/blob/master/src/GUI/EFCorePowerTools/Helpers/EnvDTEHelper.cs#L244
The TableInformationModel doesn't seem to allow for a View property to indicate it is a view and not a table?

@ErikEJ
Copy link
Owner Author

ErikEJ commented Aug 10, 2019

It has a HasPrimaryKey property => views have no primary key!

@jespersh
Copy link
Contributor

It has a HasPrimaryKey property => views have no primary key!

So what do you wish for to do with tables without a primary key?
I am currently treating primary-keyless tables and views the same.

@ErikEJ
Copy link
Owner Author

ErikEJ commented Aug 10, 2019

The can now be scaffoled as Entities with not key (HasNoKey)

@roji
Copy link

roji commented Aug 10, 2019

@ErikEJ can you please provide more details on exactly what you need? Npgsql exposes view information via the "views" schema collection, in a way that seems to be compatible with the way SqlClient does it. Let me know what exactly is missing and I'll look into it.

@ErikEJ
Copy link
Owner Author

ErikEJ commented Aug 10, 2019

@roji thanks, I will just implement the changes myself

@roji
Copy link

roji commented Aug 10, 2019

Ok @ErikEJ, let me know if you need anything from my side.

ErikEJ pushed a commit that referenced this issue Oct 4, 2019
@ErikEJ
Copy link
Owner Author

ErikEJ commented Oct 4, 2019

@roji Would be grateful if you could check (review) this! f67ab72 (I am using 4.0.6 now)

ErikEJ added a commit that referenced this issue Oct 4, 2019
@roji
Copy link

roji commented Oct 5, 2019

@ErikEJ yep, that looks like it should work! Let me know if you hit any issues with it.

@ErikEJ
Copy link
Owner Author

ErikEJ commented Oct 5, 2019

@roji thanks, I will let some Postgres users test it!

@ErikEJ
Copy link
Owner Author

ErikEJ commented Oct 5, 2019

@roji Any ideas in relation to this: "// TODO: Check if the table has a primary key" ?

@roji
Copy link

roji commented Oct 5, 2019

Hmm... I don't know exactly what your code does, but you can have tables with and without primary keys - that may be important (or not). However, AFAIK in PG views cannot have primary keys (or any kind of constraint), so this may just be a copy-paste.

@ErikEJ
Copy link
Owner Author

ErikEJ commented Oct 5, 2019

@roji I mean: how do I detect if a table (not a view) has a primary key?

@roji
Copy link

roji commented Oct 5, 2019

I think you need to get a different metadata schema. Looking at the source, getting "CONSTRAINTS" or possibly "CONSTRAINTCOLUMNS" should be it (not in front of my computer so can't dig deeper)...

@ErikEJ
Copy link
Owner Author

ErikEJ commented Oct 7, 2019

@roji are you able to get the latest daily and test this? https://github.com/ErikEJ/EFCorePowerTools/blob/master/src/GUI/EFCorePowerTools/Helpers/EnvDTEHelper.cs#L203 (I do not have a server installed)

@roji
Copy link

roji commented Oct 7, 2019

@ErikEJ I can try to find some time during this week, but it's gonna be tight (dotnetos conference). But it's super easy to install PostgreSQL and test stuff: https://www.postgresql.org/download/windows, I'd rather help you do that :trollface:

@jespersh
Copy link
Contributor

jespersh commented Oct 7, 2019

Seems like I can only select View/Tables without primary keys right now
Version: 2.3.47.0
Is that the newest version? I can't tell what was fixed/changed in what version

image

@ErikEJ
Copy link
Owner Author

ErikEJ commented Oct 7, 2019

@jespersh That looks like a bug in the latest daily build! (I am working on EF Core 3.0 support)

@ErikEJ
Copy link
Owner Author

ErikEJ commented Oct 7, 2019

Should be fixed now (2.3.48 or later)

@ErikEJ
Copy link
Owner Author

ErikEJ commented Oct 7, 2019

@jespersh I really appreciate that you are running the latest daily build!

@jespersh
Copy link
Contributor

jespersh commented Oct 7, 2019

I tried with 2.3.49.0 and got the same window. Can't select regular tables with primary keys.
This IsEnabled

IsEnabled="{Binding Model.HasKey}"/>

should:

  • always be true for 3.0
  • only be true for tables with primary keys for 2.x

@ErikEJ
Copy link
Owner Author

ErikEJ commented Oct 7, 2019

That is correct! What am I doing wrong?

@ErikEJ
Copy link
Owner Author

ErikEJ commented Oct 7, 2019

I think I found the bug! 😊

@jespersh
Copy link
Contributor

jespersh commented Oct 7, 2019

Wait, are these new?

public bool HasKey
{
get => _isKeyLess;
set
{
if (value == _isKeyLess) return;
_isKeyLess = value;
OnPropertyChanged();
}
}

I notice that "_isKeyless" is the inverse of "HasKey", so which is it? :-)
I think your issue might be some flipping of the bool

For mysql am I just setting

result.Add(new TableInformationModel(table, hasPrimaryKey));

@jespersh
Copy link
Contributor

jespersh commented Oct 7, 2019

And you can delete these lines:

var info = new TableInformationModel(table, hasPrimaryKey)
{
HasKey = includeViews ? true : hasPrimaryKey
};

They appear to have no effect and I don't remember why I put it twice. Think a mistake was made that I always included views in the list, but it never bothered me.

@jespersh
Copy link
Contributor

jespersh commented Oct 7, 2019

Just noticed you added it yourself :-)
8d40124

info is unused.
Maybe it'll just work if you replace
result.Add(new TableInformationModel(table, hasPrimaryKey));
with
result.Add(info);

@ErikEJ
Copy link
Owner Author

ErikEJ commented Oct 7, 2019

@jespersh Does the latest daily fix it?

@jespersh
Copy link
Contributor

jespersh commented Oct 7, 2019

@ErikEJ I don't see a mysql related commit after #233 (comment)
I can test again if you think I should

@ErikEJ
Copy link
Owner Author

ErikEJ commented Oct 7, 2019

Doh - never pushed the fix - just did it now!

@jespersh
Copy link
Contributor

jespersh commented Oct 8, 2019

It works again. Thanks!

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

Successfully merging a pull request may close this issue.

3 participants