-
Notifications
You must be signed in to change notification settings - Fork 751
Add domain models and review OQL documentation #9914
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
base: development
Are you sure you want to change the base?
Conversation
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.
First shot at adding a domain model and doing a quick review.
SELECT Sales.Request/* | ||
FROM Sales.Customer | ||
JOIN Sales.Customer/Sales.Customer_Request/Sales.Request | ||
JOIN Sales.Customer/Sales.Request_Customer/Sales.Request |
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.
I think this was back to front naming?
|
||
```sql | ||
SELECT Revenue, Cost FROM Sales.Finance WHERE Revenue IS NOT NULL | ||
SELECT Revenue, Cost FROM Sales.Finances WHERE Revenue IS NOT NULL |
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.
Typo - the other examples used "Finances"
``` | ||
|
||
| ID | Import | | ||
| ID | RawImport | |
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.
"Import" is an invalid attribute name
|
||
```sql | ||
SELECT HighestStockProductName FROM Sales.Product | ||
SELECT Name AS HighestStockProductName FROM Sales.Product |
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.
I think we want to pick up Name here but report it as "HighestStockProductName"
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.
Not sure about this - what is the relationship between Storage and Location?
I think the example works with this, but not convinced that the example gives a meaningful answer?
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.
The idea was to have something meaningful from the OQL perspective. We could add an association between those, but I would not since it is not used in the queries.
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.
The Customer entity is mostly defined with LastName
/FirstName
in the examples. Only a few examples on Line 789 breaks that an includes Age
and TotalOrderAmount
. Perhaps let's rename the entity there to something else like CustomerInfo
and use it in the COALESCE
examples
No description provided.