Query cleanup #17130
Query cleanup #17130
Conversation
| @@ -51,8 +51,6 @@ public ExpressionPrinter() | |||
| _encounteredParameters = new List<ParameterExpression>(); | |||
| } | |||
|
|
|||
| public virtual IndentedStringBuilder StringBuilder => _stringBuilder; | |||
There was a problem hiding this comment.
Breaking change. Use API on expression printer directly
| @@ -101,18 +99,6 @@ public virtual ExpressionPrinter AppendLines([NotNull] object o, bool skipFinalN | |||
| return this; | |||
| } | |||
|
|
|||
| public virtual ExpressionPrinter IncrementIndent() | |||
There was a problem hiding this comment.
Remove Increment/Decrement Indent. Using Indent and disposable pattern.
smitpatel
force-pushed
the
smit/somerenames
branch
from
d309c50
to
0263958
Compare
|
@smitpatel I intentionally made the factory public because we have dependencies for it, so it makes sense for providers to extend from our factory. |
| query = new NullComparisonTransformingExpressionVisitor().Visit(query); | ||
|
|
||
| return query; | ||
| } | ||
|
|
||
| protected virtual Expression OptimizeSqlExpression(Expression query) => _sqlExpressionOptimizingExpressionVisitor.Visit(query); |
There was a problem hiding this comment.
All visitors in optimizers are supposed to be internal and exposed via protected methods. This particular visitor moved to .Internal but being used by SqlServer so I have added method to facilitate this for now. We can go over and add methods to call other visitors too in future.
|
@ajcvickers - I will check and revert if necessary. |
| /// <value> | ||
| /// The execution strategy factory. | ||
| /// </value> | ||
| protected virtual IExecutionStrategyFactory ExecutionStrategyFactory |
There was a problem hiding this comment.
Factory should not be exposing anything from Dependency object.
| @@ -11,9 +11,5 @@ public interface IParameterValues | |||
| IReadOnlyDictionary<string, object> ParameterValues { get; } | |||
|
|
|||
| void Add([NotNull] string name, [CanBeNull] object value); | |||
|
|
|||
| object Remove([NotNull] string name); | |||
There was a problem hiding this comment.
These APIs were used in old pipeline. Not anymore. It should not affect any provider.
| /// any release. You should only use it directly in your code with extreme caution and knowing that | ||
| /// doing so can result in application failures when updating to a new Entity Framework Core release. | ||
| /// </summary> | ||
| public object GetValue(int index) => _values[index]; |
There was a problem hiding this comment.
This was to support include in old pipeline. No longer needed.
|
Added another commit to
|
|
We will go through breaking changes in API review again. 🎉 |
ghost
deleted the
Most of changes are renames and moving files to Internal folder. I will comment on the breaking changes for visibility.