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

Query: Allow store types and type facets to be set explicitly (EF.StoreType) #4978

Open
smitpatel opened this issue Apr 4, 2016 · 16 comments

Comments

@smitpatel
Copy link
Member

#4937 introduced logic to infer Unicode-ness for literals when comparing to column. It works well for simple cases when one side of comparison is ColumnExpression. Though there are complex cases in which we are not inferring the information correctly atm. For e.g.

  1. If one side of binary expression is subquery selecting one element of a column then the other side should have same facets as the column being projected in subquery.
  2. Operators like concat which return string type values should pass uniform information towards parent because the siblings of concat should have same unicode-ness as concat. (See test Non_unicode_string_literals_is_used_for_non_unicode_column_with_concat)

For cases like above, we can improve logic to propagate information. If it is too hard to do then we should add functions like AsUnicode & AsNonUnicode.

@smitpatel
Copy link
Member Author

Also cases of SqlFunction like CharIndex (comes from String.Contains method) the argument should have same type mapping. Though That could be different for individual functions.

@ajcvickers ajcvickers changed the title Query: Infer unicode-ness for literals in complex cases Query: Allow type facets to be set explicitly and/or improve inference in complex cases Jun 19, 2017
@ajcvickers
Copy link
Member

ajcvickers commented Jun 19, 2017

Note: when designing/implementing this, consider the case for parameters passed to FromSql, etc. See #8895
Added comment to that issue.

@ajcvickers ajcvickers changed the title Query: Allow type facets to be set explicitly and/or improve inference in complex cases Query: Allow store types and type facets to be set explicitly and/or improve inference in complex cases May 7, 2018
@ajcvickers
Copy link
Member

See also comments in #11921

@ajcvickers
Copy link
Member

See also comments in the thread here: #6717 (comment)

@roji
Copy link
Member

roji commented Aug 2, 2018

Another issue which shows the need for a way to explicitly specify a store type: npgsql/efcore.pg#561.

@roji
Copy link
Member

roji commented Aug 6, 2018

Nothing you're saying is stupid, but it's not exactly right either :)

DbType is a database-independent ADO.NET enum property on DbParameter. If works great if you want to use some universal type like int or string, which all databases support. However, once you need to use any type that's database-specific, the DbType enum isn't sufficient anymore. This is why each ADO.NET provider has its own enum (SqlDbType, NpgsqlDbType...) to allow specifying database-specific types.

The Npgsql EF Core provider doesn't use DbType at all, because it doesn't need any sort of database portability (it works only with Npgsql/PostgreSQL anyway), so it only uses NpgsqlDbType. EF Core doesn't know about about NpgsqlDbType when logging, so all you get is DbType=Object.

To summarize, Npgsql always sends an explicit database type ("type OID" in PostgreSQL parlance) with every parameter it sends. The type OID sent can be controlled via DbType, NpgsqlDbType, DataTypeName, or you can just let Npgsql use its default by not specifying any of those. But the current implementation will always tell PostgreSQL exactly what the data type is, thus preventing inference/coercion.

You can read about DbType and NpgsqlDbType in the Npgsql basic usage docs.

Note that it's still possible for EF Core to do better inference, i.e. select a better mapping based on the query context, thus making Npgsql send a different (and more correct) data type OID.

@dpsenner
Copy link

dpsenner commented Aug 7, 2018

Thanks @roji for your valuable insights. To me DbType=Object was an indicator that there's something fishy going on. I'm glad that I was wrong. Keep up your good work!

@smuthuvels
Copy link

smuthuvels commented Aug 16, 2018

I am using EF Core 2.0.3 and I am getting the same issue for Contains search. Do you have any fix for that?

@smitpatel
Copy link
Member Author

blocked on #13192

@bricelam
Copy link
Contributor

I added a mechanism for query translators to provide explicit type mappings for function arguments and results in PR #13648. This allows us to fix individual cases.

@bricelam bricelam removed their assignment Oct 17, 2018
@ajcvickers ajcvickers modified the milestones: 3.0.0, Backlog Jan 28, 2019
@smitpatel smitpatel removed the blocked label Jun 6, 2019
@smitpatel smitpatel removed this from the Backlog milestone Jun 6, 2019
@ajcvickers ajcvickers added this to the Backlog milestone Jun 6, 2019
@ajcvickers ajcvickers removed their assignment Jun 6, 2019
@smitpatel smitpatel changed the title Query: Allow store types and type facets to be set explicitly and/or improve inference in complex cases Query: Allow store types and type facets to be set explicitly Nov 13, 2019
@roji
Copy link
Member

roji commented Dec 2, 2019

Another possible need for this: setting facets in order to allow something to participate in a set operation (currently set operations over types with differing facets isn't supported, #19129).

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

7 participants