-
Notifications
You must be signed in to change notification settings - Fork 461
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
[WIP] Improved SQL generation by setting correct aliases. #1405
Conversation
/// Set this value to <c>null</c> to disable special alias generation queries. | ||
/// </remarks> | ||
/// </summary> | ||
public static string AssociationAlias { get; set; } = "A_{0}"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public static string AssociationAlias { get; set; } = "A_{0}"; | |
public static string AssociationAlias { get; set; } = "a_{0}"; |
/// </code> | ||
/// </example> | ||
/// </summary> | ||
public static bool GenerateFinalAliases { get; set; } = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public static bool GenerateFinalAliases { get; set; } = true; | |
public static bool GenerateFinalAliases { get; set; } = false; |
@@ -1050,7 +1050,7 @@ public void SetAlias(string alias) | |||
if (alias == null) | |||
return; | |||
|
|||
if (alias.Contains('<')) | |||
if (!alias.Contains('<')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it was a bug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strange but it will not allow aliasing. For me it looks like a bug because here we are trying do not use anonymous type name but in wrong way.
|
||
public void RemoveAlias(string alias) | ||
{ | ||
if (_aliases != null) | ||
{ | ||
alias = alias.ToUpper(); | ||
if (_aliases.ContainsKey(alias)) | ||
if (_aliases.Contains(alias)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (_aliases.Contains(alias)) |
You do not need to check out if you use HashSet.
case QueryElementType.SqlParameter: | ||
{ | ||
var p = (SqlParameter)expr; | ||
|
||
if (p.IsQueryParameter) | ||
{ | ||
if (!objs.ContainsKey(expr)) | ||
if (!paramsVisited.Contains(expr)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!paramsVisited.Contains(expr)) | |
if (!paramsVisited.Add(expr)) |
{ | ||
objs.Add(expr, expr); | ||
p.Name = GetAlias(p.Name, "p"); | ||
paramsVisited.Add(p); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
paramsVisited.Add(p); |
T4 will be fixed as part of #1433 |
And how I can set it back to the old |
Checked out and it seems, that the first alias wins.
|
It will be a problem even in future. It is bad to check generated SQL in tests. Especially when we apply additional optimizations. |
It's just an integration test to evaluate the lib on top we built our application. Small changes are not a problem, but this change was hard to understand in the method-syntax and is rather a configuration thing as a optimization for me. |
We have implemented this feature to simplify understanding queries. Feel free to open issue when we can improve aliasing. Samples are wellcome. |
Improved Aliases for SQL queries.
AliasName
forAssociationAttribute
.Sample query before fix:
After fix
To be done:
MakeUniqueNames
function.