-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Implemement set operation query support for complex JSON #36417
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -375,13 +375,10 @@ protected override ShapedQueryExpression TransformJsonQueryToTable(JsonQueryExpr | |
| var jsonColumn = selectExpression.CreateColumnExpression( | ||
| jsonEachExpression, JsonEachValueColumnName, typeof(string), _typeMappingSource.FindMapping(typeof(string))); // TODO: nullable? | ||
|
|
||
| var containerColumnName = structuralType.GetContainerColumnName(); | ||
| Check.DebugAssert(containerColumnName is not null, "JsonQueryExpression to entity type without a container column name"); | ||
|
|
||
| // First step: build a SelectExpression that will execute json_each and project all properties and navigations out, e.g. | ||
| // (SELECT value ->> 'a' AS a, value ->> 'b' AS b FROM json_each(c."JsonColumn", '$.Something.SomeCollection') | ||
|
|
||
| // We're only interested in properties which actually exist in the JSON, filter out uninteresting shadow keys | ||
| // We're only interested in properties which actually exist in the JSON, filter out uninteresting synthetic keys | ||
| foreach (var property in structuralType.GetPropertiesInHierarchy()) | ||
| { | ||
| if (property.GetJsonPropertyName() is string jsonPropertyName) | ||
|
|
@@ -393,14 +390,14 @@ protected override ShapedQueryExpression TransformJsonQueryToTable(JsonQueryExpr | |
|
|
||
| propertyJsonScalarExpression[projectionMember] = new JsonScalarExpression( | ||
| jsonColumn, | ||
| new[] { new PathSegment(property.GetJsonPropertyName()!) }, | ||
| [new PathSegment(property.GetJsonPropertyName()!)], | ||
| property.ClrType.UnwrapNullableType(), | ||
| property.GetRelationalTypeMapping(), | ||
| property.IsNullable); | ||
| } | ||
| } | ||
|
|
||
| if (jsonQueryExpression.StructuralType is IEntityType entityType) | ||
| if (structuralType is IEntityType entityType) | ||
| { | ||
| foreach (var navigation in entityType.GetNavigationsInHierarchy() | ||
| .Where( | ||
|
|
@@ -422,7 +419,20 @@ [new PathSegment(jsonNavigationName)], | |
| } | ||
| } | ||
|
|
||
| // TODO: also add JsonScalarExpressions for complex properties, not just owned entities. #36296. | ||
| foreach (var complexProperty in structuralType.GetComplexProperties()) | ||
| { | ||
| var jsonNavigationName = complexProperty.ComplexType.GetJsonPropertyName(); | ||
| Check.DebugAssert(jsonNavigationName is not null, "Invalid complex property found on JSON-mapped structural type"); | ||
|
|
||
| var projectionMember = new ProjectionMember().Append(new FakeMemberInfo(jsonNavigationName)); | ||
|
|
||
| propertyJsonScalarExpression[projectionMember] = new JsonScalarExpression( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's strange that the type is called
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tell me about it - this prompted me to file #36392, which contains the details. tl;dr JsonQueryExpression doesn't actually represent JSON_QUERY() in SQL, but rather represents a JSON being projected out of the database (so shaper-side expression only). Whe projections are applied (subquery pushdown, or when translation ends), any projected JsonQueryExpression gets converted to JsonScalarExpression. ... very questionable. |
||
| jsonColumn, | ||
| [new PathSegment(jsonNavigationName)], | ||
| typeof(string), | ||
| textTypeMapping, | ||
| jsonQueryExpression.IsNullable || complexProperty.IsNullable); | ||
| } | ||
|
|
||
| selectExpression.ReplaceProjection(propertyJsonScalarExpression); | ||
|
|
||
|
|
||
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.
containerColumnNamedoes not seem to be used outside of the Assert.The comment
// We're only interested in properties which actually exist in the JSON, filter out uninteresting shadow keysis incorrect as you aren't filtering out shadow properties. Also, invert the condition to reduce nestingThere 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.
Re shadow keys/properties, the intent here was to mean that we want to jump over the synthetic key properties, which don't actually exist in the JSON document - that's what the
if (property.GetJsonPropertyName() is string jsonPropertyName)does.I'll change the comment to
filter out uninteresting synthetic keysto make it clearer.