Support more operators for the on condition #99
Conversation
maiargu
force-pushed
the
exptend-operators
branch
from
81502b0 to
da015ec
Compare
| examples: | ||
| - [{ "ref": ["to_Connection", "AirlineID"] }, "=", { "ref": ["AirlineID"] }] | ||
| - [{ "ref": ["to_Connection", "FlightDate"] }, "<=", { "ref": ["FlightDate"] }] | ||
| - [{ "ref": ["to_Connection", "FlightDate"] }, ">=", { "ref": ["FlightDate"] }] |
There was a problem hiding this comment.
Some questions from my side, to be discussed:
- if the operators "<=" and "<=" are allowed, are they restricted only to some specific
cds.type operands?
Example, "<=" and "<=" operators :
- allowed for operands of type "cds.Integer" | "cds.Integer64" | "cds.Decimal" | "cds.Double" | "cds.Date" | "cds.Time" | "cds.DateTime" | "cds.Timestamp" ?
- but forbidden for operands of type "cds.Boolean" | "cds.String" | "cds.LargeString" | "cds.UUID"?
- Who does the check if the two operands are of the same type and it makes sense to combine them via an operator sign?
There was a problem hiding this comment.
-
Yes, I'm pretty sure this is the case, and we need to spell this out somewhere. Maybe we should create a section in the Primer or somewhere else and link to it from the interface docu. We may even have a "sub-specification" here, where we need to explain how "on" condition queries work.
-
@stewsk - do you know if this is something CAP compiler checks? It would be possible to check via the metadata validator, but writing that validation code will be a bit more involved.
There was a problem hiding this comment.
Hi @Fannon , CAP doesn't apply any restrictions on the ON condition. Any kind of condition (all operators, all types) are allowed. Accordingly, we don't have any such checks.
There was a problem hiding this comment.
What happens if an operator is not known / supported by CAP or another consumer?
There was a problem hiding this comment.
But wouldn't this on condition need to be resolved, in the case of an association even as a foreign key?
Maybe the example added here is a bit confusing. An "on" condition of an association or composition probably always need to return exactly one match? Its a different use case compared to a view where we want to query / select multiple results. Here comparator operations like >= make more sense to me, but we don't have Views / Queries like that in CSN Interop.
| The `on` condition is constructed by triples of: | ||
| - Reference to the target element (ID) as array with 2 items | ||
| - Equals Operator "=" | ||
| - Equals Operator `=` or Smaller Equals Operator `<=` or Bigger Equals Operator `>=` |
There was a problem hiding this comment.
| - Equals Operator `=` or Smaller Equals Operator `<=` or Bigger Equals Operator `>=` | |
| - Operator: Equals `=`, Smaller Equals`<=` or Bigger Equals `>=` |
|
Let's either add it to the spec itself and then update the existing check, or maybe it's a non-issue anyway because the validation result was correct and the CSN files have to be fixed. |
|
Sorry, I closed the wrong PR. Let's keep this open until we clarified this. |
| The operator `=` | ||
| const: "=" | ||
|
|
||
| SmallerEqualsOperator: |
There was a problem hiding this comment.
If we use smaller or bigger equals operators, the cardinality of the "on" condition has to be "to many".
We need to define for which data types a comparator like this is valid and how comparison works. So far we seem to only need it for date (time dependent references / queries)
| The operator `<=` | ||
| const: "<=" | ||
|
|
||
| BiggerEqualsOperator: |
There was a problem hiding this comment.
| BiggerEqualsOperator: | |
| GreaterEqualsOperator: |
Fannon
changed the title
PR is in draft mode, the changes to be discussed.