Skip to content
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

clarify type casting in CESQL spec #1281

Merged
15 changes: 15 additions & 0 deletions cesql/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,10 @@ For example, the pattern `_b*` will accept values `ab`, `abc`, `abcd1` but won't
Both `%` and `_` can be escaped with `\`, in order to be matched literally. For example, the pattern `abc\%` will match
`abc%` but won't match `abcd`.

In cases where the left operand is not a `String`, it MUST be cast to a `String` before the comparison is made.
The pattern of the `LIKE` operator (that is, the right operand of the operator) MUST be a valid string predicate,
otherwise the parse MUST return a parse error.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be clear... subject LIKE TRUE isn't valid because TRUE isn't a valid string, however, we talk about how there's implicit casting all over the place... so someone may wonder why TRUE isn't implicitly converted into "true". Am I correct in this thinking? Should we be explicit and say that casting isn't allowed in this case? Or perhaps should we allow it for consistency? @jskeet thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would definitely be good to be clear. Even without the "LIKE" aspect, I can't tell offhand whether "TRUE" = true is casting the LHS to Boolean, or the RHS to String. I suspect it's the former, based on bullet 2 in the list in 3.7.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@duglin I think the main problem is that currently the LIKE expression isn't defined with an expression as the right operand. It is defined as:

expression NOT? LIKE stringLiteral

So, any value on the right operand which is not a string literal MUST be a parse error currently. We can change this, but I'm not sure if it makes sense. The whole point of a LIKE expression is to compare against a pattern, not a specific value. If someone were to compare against another string value wouldn't it just makes sense to use = or something like that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type casting is whenever an operator is defined for values, but the LIKE operator is currently only defined for a literal, not a value. I'm not 100% sure why this was the initial decision, but that's how it currently works

Copy link
Collaborator

@duglin duglin May 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's ok to leave it as "must be a string literal w/o casting", but let's be extra clear in the text that implicit type casting isn't allowed in this one spot. I agree defining it as stringLiteral sort of implies it, but saying that things should be implicitly case in all other spaces (even when the spec says the arg is a string) could be unclear to people.


#### 3.4.4. Exists operator

| Definition | Semantics |
Expand Down Expand Up @@ -378,6 +382,9 @@ A CESQL engine MUST apply the following implicit casting rules in order:

For the `IN` operator, a special rule is defined: the left argument MUST be used as the target type to eventually cast the set elements.

For _Boolean_ values being cast to _String_, the resulting string MUST be lowercase. That is `true` becomes `"true"` and `false` becomes `"false"`.
Similarly, `TRUE` becomes `"true"` and `FALSE` becomes `"false"`.

For example, assuming `MY_STRING_PREDICATE` is a unary predicate accepting a _String_ parameter and returning a
_Boolean_, this expression:

Expand All @@ -402,6 +409,14 @@ sequence = 10
`Integer x Integer`. Because the right operand of the operator is an _Integer_ and there is only one `=` definition
which uses the type _Integer_ as the right parameter, `sequence` is cast to _Integer_.

A CESQL engine MUST support the following type casts:

1. `Integer -> String`
1. `String -> Integer`
1. `String -> Boolean`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where are ones like this defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I had assumed that they were defined elsewhere and I was just seeking to clarify which ones had to be supported. But, looking more closely now I don't see those defined anywhere...

Let me add those

1. `Boolean -> String`


## 4. Implementation suggestions

This section is meant to provide some suggestions while implementing and adopting the CloudEvents Expression Language.
Expand Down
Loading