addressing final API review feedback for json columns (#28820)

src/EFCore.Relational/Query/JsonQueryExpression.cs

@@ -33,7 +33,7 @@ public JsonQueryExpression(
  collection,
  keyPropertyMap,
  type,
- jsonPath: new SqlConstantExpression(Constant("$"), typeMapping: null),
+ path: new SqlConstantExpression(Constant("$"), typeMapping: null),
  jsonColumn.IsNullable)
  {
  }
@@ -44,7 +44,7 @@ private JsonQueryExpression(
  bool collection,
  IReadOnlyDictionary<IProperty, ColumnExpression> keyPropertyMap,
  Type type,
- SqlExpression jsonPath,
+ SqlExpression path,
  bool nullable)
  {
  Check.DebugAssert(entityType.FindPrimaryKey() != null, "primary key is null.");
@@ -54,7 +54,7 @@ private JsonQueryExpression(
  IsCollection = collection;
  _keyPropertyMap = keyPropertyMap;
  Type = type;
- JsonPath = jsonPath;
+ Path = path;
  _nullable = nullable;
  }
 
@@ -76,7 +76,7 @@ private JsonQueryExpression(
  /// <summary>
  /// The JSON path leading to the entity from the root of the JSON stored in the column.
  /// </summary>
- public virtual SqlExpression JsonPath { get; }
+ public virtual SqlExpression Path { get; }
 
  /// <summary>
  /// The value indicating whether this expression is nullable.
@@ -112,7 +112,7 @@ public virtual SqlExpression BindProperty(IProperty property)
 
  var newPath = new SqlBinaryExpression(
  ExpressionType.Add,
- JsonPath,
+ Path,
  pathSegment,
  typeof(string),
  typeMapping: null);
@@ -148,7 +148,7 @@ public virtual JsonQueryExpression BindNavigation(INavigation navigation)
 
  var newJsonPath = new SqlBinaryExpression(
  ExpressionType.Add,
- JsonPath,
+ Path,
  pathSegment,
  typeof(string),
  typeMapping: null);
@@ -190,27 +190,26 @@ public virtual JsonQueryExpression MakeNullable()
  /// Makes this JSON query expression nullable re-using existing nullable key properties
  /// </summary>
  /// <returns>A new expression which has <see cref="IsNullable" /> property set to true.</returns>
- [EntityFrameworkInternal]
- public virtual JsonQueryExpression MakeNullable(IReadOnlyDictionary<IProperty, ColumnExpression> nullableKeyPropertyMap)
+ internal virtual JsonQueryExpression MakeNullable(IReadOnlyDictionary<IProperty, ColumnExpression> nullableKeyPropertyMap)
  => Update(
  JsonColumn.MakeNullable(),
  nullableKeyPropertyMap,
- JsonPath,
+ Path,
  nullable: true);
 
  /// <inheritdoc />
  public virtual void Print(ExpressionPrinter expressionPrinter)
  {
  expressionPrinter.Append("JsonQueryExpression(");
  expressionPrinter.Visit(JsonColumn);
- expressionPrinter.Append($", \"{string.Join(".", JsonPath)}\")");
+ expressionPrinter.Append($", \"{string.Join(".", Path)}\")");
  }
 
  /// <inheritdoc />
  protected override Expression VisitChildren(ExpressionVisitor visitor)
  {
  var jsonColumn = (ColumnExpression)visitor.Visit(JsonColumn);
- var jsonPath = (SqlExpression)visitor.Visit(JsonPath);
+ var jsonPath = (SqlExpression)visitor.Visit(Path);
 
  // TODO: also visit columns in the _keyPropertyMap?
  return Update(jsonColumn, _keyPropertyMap, jsonPath, IsNullable);
@@ -222,19 +221,19 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
  /// </summary>
  /// <param name="jsonColumn">The <see cref="JsonColumn" /> property of the result.</param>
  /// <param name="keyPropertyMap">The map of key properties and columns they map to.</param>
- /// <param name="jsonPath">The <see cref="JsonPath" /> property of the result.</param>
+ /// <param name="path">The <see cref="Path" /> property of the result.</param>
  /// <param name="nullable">The <see cref="IsNullable" /> property of the result.</param>
  /// <returns>This expression if no children changed, or an expression with the updated children.</returns>
  public virtual JsonQueryExpression Update(
  ColumnExpression jsonColumn,
  IReadOnlyDictionary<IProperty, ColumnExpression> keyPropertyMap,
- SqlExpression jsonPath,
+ SqlExpression path,
  bool nullable)
  => jsonColumn != JsonColumn
  || keyPropertyMap.Count != _keyPropertyMap.Count
  || keyPropertyMap.Zip(_keyPropertyMap, (n, o) => n.Value != o.Value).Any(x => x)
- || jsonPath != JsonPath
- ? new JsonQueryExpression(EntityType, jsonColumn, IsCollection, keyPropertyMap, Type, jsonPath, nullable)
+ || path != Path
+ ? new JsonQueryExpression(EntityType, jsonColumn, IsCollection, keyPropertyMap, Type, path, nullable)
  : this;
 
  /// <inheritdoc />
@@ -248,7 +247,7 @@ private bool Equals(JsonQueryExpression jsonQueryExpression)
  => EntityType.Equals(jsonQueryExpression.EntityType)
  && JsonColumn.Equals(jsonQueryExpression.JsonColumn)
  && IsCollection.Equals(jsonQueryExpression.IsCollection)
- && JsonPath.Equals(jsonQueryExpression.JsonPath)
+ && Path.Equals(jsonQueryExpression.Path)
  && IsNullable == jsonQueryExpression.IsNullable
  && KeyPropertyMapEquals(jsonQueryExpression._keyPropertyMap);
 
@@ -273,6 +272,6 @@ private bool KeyPropertyMapEquals(IReadOnlyDictionary<IProperty, ColumnExpressio
  /// <inheritdoc />
  public override int GetHashCode()
  // not incorporating _keyPropertyMap into the hash, too much work 
- => HashCode.Combine(EntityType, JsonColumn, IsCollection, JsonPath, IsNullable);
+ => HashCode.Combine(EntityType, JsonColumn, IsCollection, Path, IsNullable);
  }
 }

src/EFCore.Relational/Query/SqlExpressions/JsonScalarExpression.cs

@@ -13,27 +13,27 @@ public class JsonScalarExpression : SqlExpression
  /// </summary>
  /// <param name="jsonColumn">A column containg JSON.</param>
  /// <param name="property">A property representing the result of this expression.</param>
- /// <param name="jsonPath">A JSON path leading to the scalar from the root of the JSON stored in the column.</param>
+ /// <param name="path">A JSON path leading to the scalar from the root of the JSON stored in the column.</param>
  /// <param name="nullable">A value indicating whether the expression is nullable.</param>
  public JsonScalarExpression(
  ColumnExpression jsonColumn,
  IProperty property,
- SqlExpression jsonPath,
+ SqlExpression path,
  bool nullable)
- : this(jsonColumn, property.ClrType, property.FindRelationalTypeMapping()!, jsonPath, nullable)
+ : this(jsonColumn, property.ClrType, property.FindRelationalTypeMapping()!, path, nullable)
  {
  }
 
  internal JsonScalarExpression(
  ColumnExpression jsonColumn,
  Type type,
  RelationalTypeMapping typeMapping,
- SqlExpression jsonPath,
+ SqlExpression path,
  bool nullable)
  : base(type, typeMapping)
  {
  JsonColumn = jsonColumn;
- JsonPath = jsonPath;
+ Path = path;
  IsNullable = nullable;
  }
 
@@ -45,7 +45,7 @@ internal JsonScalarExpression(
  /// <summary>
  /// The JSON path leading to the scalar from the root of the JSON stored in the column.
  /// </summary>
- public virtual SqlExpression JsonPath { get; }
+ public virtual SqlExpression Path { get; }
 
  /// <summary>
  /// The value indicating whether the expression is nullable.
@@ -63,7 +63,7 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
  jsonColumn,
  Type,
  TypeMapping!,
- JsonPath,
+ Path,
  IsNullable || jsonColumnMadeNullable)
  : this;
  }
@@ -73,17 +73,14 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
  /// return this expression.
  /// </summary>
  /// <param name="jsonColumn">The <see cref="JsonColumn" /> property of the result.</param>
- /// <param name="jsonPath">The <see cref="JsonPath" /> property of the result.</param>
- /// <param name="nullable">The <see cref="IsNullable" /> property of the result.</param>
+ /// <param name="path">The <see cref="Path" /> property of the result.</param>
  /// <returns>This expression if no children changed, or an expression with the updated children.</returns>
  public virtual JsonScalarExpression Update(
  ColumnExpression jsonColumn,
- SqlExpression jsonPath,
- bool nullable)
+ SqlExpression path)
  => jsonColumn != JsonColumn
- || jsonPath != JsonPath
- || nullable != IsNullable
- ? new JsonScalarExpression(jsonColumn, Type, TypeMapping!, jsonPath, nullable)
+ || path != Path
+ ? new JsonScalarExpression(jsonColumn, Type, TypeMapping!, path, IsNullable)
  : this;
 
  /// <inheritdoc />
@@ -92,18 +89,18 @@ protected override void Print(ExpressionPrinter expressionPrinter)
  expressionPrinter.Append("JsonScalarExpression(column: ");
  expressionPrinter.Visit(JsonColumn);
  expressionPrinter.Append(" Path: ");
- expressionPrinter.Visit(JsonPath);
+ expressionPrinter.Visit(Path);
  expressionPrinter.Append(")");
  }
 
  /// <inheritdoc />
  public override bool Equals(object? obj)
  => obj is JsonScalarExpression jsonScalarExpression
  && JsonColumn.Equals(jsonScalarExpression.JsonColumn)
- && JsonPath.Equals(jsonScalarExpression.JsonPath);
+ && Path.Equals(jsonScalarExpression.Path);
 
  /// <inheritdoc />
  public override int GetHashCode()
- => HashCode.Combine(base.GetHashCode(), JsonColumn, JsonPath);
+ => HashCode.Combine(base.GetHashCode(), JsonColumn, Path);
  }
 }

src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs

@@ -1382,7 +1382,7 @@ static Dictionary<JsonQueryExpression, JsonScalarExpression> BuildJsonProjection
  {
  var ordered = projections
  .OrderBy(x => $"{x.JsonColumn.TableAlias}.{x.JsonColumn.Name}")
- .ThenBy(x => BreakJsonPathIntoComponents(x.JsonPath).Count);
+ .ThenBy(x => BreakJsonPathIntoComponents(x.Path).Count);
 
  var needed = new List<JsonScalarExpression>();
  foreach (var orderedElement in ordered)
@@ -1395,7 +1395,7 @@ static Dictionary<JsonQueryExpression, JsonScalarExpression> BuildJsonProjection
  orderedElement.JsonColumn,
  orderedElement.JsonColumn.Type,
  orderedElement.JsonColumn.TypeMapping!,
- orderedElement.JsonPath,
+ orderedElement.Path,
  orderedElement.IsNullable);
 
  needed.Add(jsonScalarExpression);
@@ -1433,8 +1433,8 @@ ConstantExpression AddJsonProjection(JsonQueryExpression jsonQueryExpression, Js
  var additionalPath = new string[0];
 
  // this will be more tricky once we support more complicated json path options
- additionalPath = BreakJsonPathIntoComponents(jsonQueryExpression.JsonPath)
- .Skip(BreakJsonPathIntoComponents(jsonScalarToAdd.JsonPath).Count)
+ additionalPath = BreakJsonPathIntoComponents(jsonQueryExpression.Path)
+ .Skip(BreakJsonPathIntoComponents(jsonScalarToAdd.Path).Count)
  .Select(x => (string)((SqlConstantExpression)x).Value!)
  .ToArray();
 
@@ -1481,8 +1481,8 @@ static bool JsonEntityContainedIn(JsonScalarExpression sourceExpression, JsonQue
  return false;
  }
 
- var sourcePath = BreakJsonPathIntoComponents(sourceExpression.JsonPath);
- var targetPath = BreakJsonPathIntoComponents(targetExpression.JsonPath);
+ var sourcePath = BreakJsonPathIntoComponents(sourceExpression.Path);
+ var targetPath = BreakJsonPathIntoComponents(targetExpression.Path);
 
  if (targetPath.Count < sourcePath.Count)
  {
@@ -3436,7 +3436,7 @@ JsonQueryExpression LiftJsonQueryFromSubquery(JsonQueryExpression jsonQueryExpre
  jsonQueryExpression.JsonColumn,
  jsonQueryExpression.JsonColumn.TypeMapping!.ClrType,
  jsonQueryExpression.JsonColumn.TypeMapping,
- jsonQueryExpression.JsonPath,
+ jsonQueryExpression.Path,
  jsonQueryExpression.IsNullable);
 
  var newJsonColumn = subquery.GenerateOuterColumn(subqueryTableReferenceExpression, jsonScalarExpression);
@@ -3461,7 +3461,7 @@ JsonQueryExpression LiftJsonQueryFromSubquery(JsonQueryExpression jsonQueryExpre
  return jsonQueryExpression.Update(
  newJsonColumn,
  newKeyPropertyMap,
- jsonPath: new SqlConstantExpression(Expression.Constant("$"), typeMapping: null),
+ path: new SqlConstantExpression(Constant("$"), typeMapping: null),
  newJsonColumn.IsNullable);
  }
  }

src/EFCore.SqlServer/Query/Internal/SearchConditionConvertingExpressionVisitor.cs

@@ -765,9 +765,9 @@ protected override Expression VisitJsonScalar(JsonScalarExpression jsonScalarExp
  {
  var parentSearchCondition = _isSearchCondition;
  _isSearchCondition = false;
- var jsonPath = (SqlExpression)Visit(jsonScalarExpression.JsonPath);
+ var jsonPath = (SqlExpression)Visit(jsonScalarExpression.Path);
  _isSearchCondition = parentSearchCondition;
 
- return jsonScalarExpression.Update(jsonScalarExpression.JsonColumn, jsonPath, jsonScalarExpression.IsNullable);
+ return jsonScalarExpression.Update(jsonScalarExpression.JsonColumn, jsonPath);
  }
 }

src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs

@@ -309,9 +309,9 @@ protected override Expression VisitJsonScalar(JsonScalarExpression jsonScalarExp
 
  var jsonPathStrings = new List<string>();
 
- if (jsonScalarExpression.JsonPath != null)
+ if (jsonScalarExpression.Path != null)
  {
- var currentPath = jsonScalarExpression.JsonPath;
+ var currentPath = jsonScalarExpression.Path;
  while (currentPath is SqlBinaryExpression sqlBinary && sqlBinary.OperatorType == ExpressionType.Add)
  {
  currentPath = sqlBinary.Left;