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

is HasComputedColumnSql bug or wrong way to use? #27814

Closed
ByZhouhang opened this issue Apr 13, 2022 · 24 comments
Closed

is HasComputedColumnSql bug or wrong way to use? #27814

ByZhouhang opened this issue Apr 13, 2022 · 24 comments

Comments

@ByZhouhang
Copy link

Include your code

public class User:
    {
        public string FirstName { get; set; }
        public string LastName { get; set; }

       // not want   store  this Column , but   need it    for  search  or  linq  Select Opt
        [NotMapped]
        public string FullName { get; set; }

    }
// false:   the value is computed when the value is read, and does not occupy any actual storage
  modelBuilder.Entity<User>().Property(t => t.FullName).HasComputedColumnSql("[LastName] + ' / ' + [FirstName]",false);

// want  wirte like this  
_userRepository.Where(t => t.FullName .Contains("CC")).First()

_userRepository.Select(t => t.FullName).First()

Include verbose output

Microsoft.Data.SqlClient.SqlException (0x80131904): 列名 'FullName' 无效。

Include provider and version information

EF Core version:
Database provider: (e.g. Microsoft.EntityFrameworkCore.SqlServer)
Target framework: (e.g. .NET 6.0)
Operating system:
IDE: (e.g. Visual Studio 2022)

@roji
Copy link
Member

roji commented Apr 15, 2022

@ByZhouhang you seem to be setting up a computed column, and then wanting to use that in queries - that should work just fine. Are you sure you actually created the computed column in the database, e.g. by running a migration?

If you're still having difficulties, please post a minimal, runnable code sample that shows the error happening.

@bricelam
Copy link
Contributor

[NotMapped] tells EF that the column doesn't exist in the database, but it sounds like you do want it to exist in the database as a computed column.

@KurogamiLight3303
Copy link

I think that approach have something wrong. U are setting the property as ignored with annotations and later making it a computed column. The computed columns are database stored. You can create a where clause with the custom search over the FirstName + LastName expresion.

@ByZhouhang
Copy link
Author

@ByZhouhang you seem to be setting up a computed column, and then wanting to use that in queries - that should work just fine. Are you sure you actually created the computed column in the database, e.g. by running a migration?

If you're still having difficulties, please post a minimal, runnable code sample that shows the error happening.

if i use DBFirst ,i must config computed column in db,not in code, is it right?

@ByZhouhang
Copy link
Author

yes, i misanderstanding the use with computed column, before i thought computed column can use but not stored in the database

@bricelam
Copy link
Contributor

I think you're looking for #10768

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
@michiproep
Copy link

michiproep commented Mar 20, 2023

Hi @ajcvickers,
I actually got the same issue (EF Core 7).
The documentation states that there two possibilities: HasComputedColumn( "xyz", store: true or false).
But for store = false, the Expression is not evaluated at all. This seems like a bug.

My actual case is even more complicated:
I need to use HasComputedColumnSql even twice on the same property:

.HasColumnName("PASSWORD")
//selectExpression
.HasComputedColumnSql($"storedProc.decrypt_password([SEEDColumn], [PASSWORD])", stored: false) 
 //insertUpdateExpression
.HasComputedColumnSql($"storedProc.encrypt_password([SEEDColumn], [PASSWORD])", stored: true)

@roji
Copy link
Member

roji commented Mar 20, 2023

@michiproep please open a new issue with a minimal, runnable code sample - it's impossible to know from the above exactly what's going on. I'd advise checking check the SQL being executed when you apply your migration.

@ajcvickers
Copy link
Contributor

@michiproep Note that if you call HasComputedColumnSql twice, then the second call overrides the first. They are not combined.

@michiproep
Copy link

@ajcvickers : Just to double check before I open a issue:
1.
I currently try to use only one "HasComputedColumnSql", using the second would be a new feature. I know that.
2.
Also, I do not have/run any migrations. The very DB exists as it is, I do not change DDL/DML. So, I just try to setup a model with fluent Api and I can see that the generated SQL does not include anything from what I set within HasComputedColumnSql("mysql", false).

So you want me to create one bug and one feature request for this?

@ajcvickers
Copy link
Contributor

@michiproep If you're not using migrations, then the equivalent change would need to be made to the database in some other way. HasComputedColumnSql tells EF that the given SQL exists on the column, and tells Migrations to create it if it doesn't exist. If the SQL doesn't exist on the column, then the runtime will act as if it does, but nothing will happen in the database.

@michiproep
Copy link

@ajcvickers : Ok, I see what you saying.
This is very confusing because with store=false I do not expect to have any column (phyiscal or virtual) on the database.
I expect that the select query transforms into something like this:
select a.Id as Id, (mycomputedSql) as ComputedProperty from myTable.

Any plans on integrating this?

I would actually give it a try and override some ExpressionVisitor if you give me hint where to start since this part is really advanced stuff ;-)

@ajcvickers
Copy link
Contributor

@michiproep Isn't that what #10768 covers?

@michiproep
Copy link

@ajcvickers : Well, this points in the right direction but
it either requires to select via FromSql() or different/anonymous type in a Select-Expression.
As a consequence, I cannot use the result obj for modifications, navigation, etc. I basically lose all the good parts.

@ajcvickers
Copy link
Contributor

@michiproep Then please file a new issue describing in detail what you want the behavior you be.

@roji
Copy link
Member

roji commented Mar 20, 2023

@michiproep #10768 would likely mean that you use SQL to define the "virtual" column (similar to how you do HasComputedColumnSql), and EF integrates that SQL into queries. The results are still materialized into your entity CLR type's property, just like a normal column.

I cannot use the result obj for modifications, navigation, etc. I basically lose all the good parts.

How would you use the results for modifications, given that the column doesn't exist in the database? Re navigations, I assume you're talking about such a virtual SQL column which returns other entities - that may be possible, or not, depending on implementation details.

@michiproep
Copy link

Hi @roji, thanks for your response.
Well, I gave my example above. This is what I'm trying to achieve:
Prerequisits:
-My customer has a user-table which may not be changed/adapted.
-Their current ORMapper allows to specify select/insert/update-expressions for a column to allow Db-side evalutions and transformations.

What I'm trying to with EFCore:

  • querying userTable
  • modify that very column while making use of the EF-integrated changeTracker
  • save back the updated entity
    -As a real use case: This password column is used for creating and updating a "shadow-OnPrem-AD-user".
    Therefore, I need to be able to "decrypt" the password while reading, changing the password while writing to DB while at the same time have all the EF features available (ChangeTracking, Concurrency, Interceptors)

I again paste my example from above:

.HasColumnName("PASSWORD") //selectExpression .HasComputedColumnSql($"storedProc.decrypt_password([SEEDColumn], [PASSWORD])", stored: false) //insertUpdateExpression .HasComputedColumnSql($"storedProc.encrypt_password([SEEDColumn], [PASSWORD])", stored: true)

Do you think I have a chance with ExpressionVisitors to achieve this?

@roji
Copy link
Member

roji commented Mar 20, 2023

I see, you want to both be able to query the column via a custom SQL (decrypt) and also save results back via another custom SQL (encrypt). #10768 indeed only covers querying. You seem to be looking for #10861, which would allow server-side value converters, wrapping values in arbitrary SQL which transform them in both directions. You are very unlikely do be able to do this yourself with expression visitors.

However, for encrypting columns this is probably not the best way; among other things, decrypting on the server (e.g. via a decrypt function) means that your values are sent unencrypted over the network. On SQL Server, you should take a look at the Always Encrypted; this doesn't require integrating decrypt/encrypt SQL around values, and also ensures that values sent over the network are encrypted.

@michiproep
Copy link

Well, I know the security issue... But that's the way it is... I don't worry too much about this since it's on an internal vlan anyways. Some addtional info on why I can't change the db: more than 1000 procedures and and about 100 applications use this table for all kind of things. We are measuring about 1k active queries per second on this table, also stateful packages/procedures which do not allow any ddl change without major downtime. Crazy stuff... Anyways, migrating old applications without this feature might be impossible then since way more tables use a similar approach...

@roji
Copy link
Member

roji commented Mar 20, 2023

There's one more way this could possibly work: you can wrap updates to the table with a stored procedure (see docs); this means all columns of the table needs to be passed to the store procedure, whose implementation can call encrypt internally only for the columns which need it. You could do the same on the read side with a table-valued function.

@michiproep
Copy link

Yes... this seems doable...
But I might still create a feature request :-)

@michiproep
Copy link

Ok, I actually managed to achive what I need by overriding the VisitColumn(ColumnExpression ce)
method and using a custom PropertyBuilder.MetaData.AddAnnotation("selectExpression").
Was some work to resolve the correct parameters within VisitColumn but now it works just fine...

@roji
Copy link
Member

roji commented Mar 20, 2023

@michiproep I'd be very wary of going down that route; doing this properly - without introducing subtle bugs - is probably going to be difficult and require good knowledge of the query pipeline (and may also involve access to internal APIs which change across releases). I'd advise on finding another way.

@michiproep
Copy link

Well, you're right but it saves me a lot of work for now.
I wish it would work out of the box

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

No branches or pull requests

6 participants