-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add EqualsAny node #262
Add EqualsAny node #262
Conversation
a4c9bcb
to
77c25f0
Compare
@@ -1000,6 +1002,10 @@ const typeRules: Dictionary<MatchFn> = { | |||
return `COALESCE(JSON_AGG(${field}), '[]')`; | |||
}, | |||
Equals: Comparison('Equals'), | |||
EqualsAny: (args, indent) => { | |||
checkArgs('EqualsAny', args, 2); | |||
return `${AnyValue(getAbstractSqlQuery(args, 0), indent)} = ANY(${AnyValue(getAbstractSqlQuery(args, 1), indent)})`; |
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.
Ideally it'd be possible to do
return `${AnyValue(getAbstractSqlQuery(args, 0), indent)} = ANY(${AnyValue(getAbstractSqlQuery(args, 1), indent)})`; | |
return `${AnyValue(getAbstractSqlQuery(args, 0), indent)} = ${typeRules.Any(getAbstractSqlQuery(args, 1), indent)}`; |
to reuse the same Any
node for the relevant part, although I'm fine with it as-is and leave that as a potential future thing
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.
@Page- They are currently fairly similar, the only reason why can't currently be done this way is because Any
requires a innerType to be casted, which I am not sure we want to infer for in
. Ofc I can rework the Any logic to be able to handle no parameter on the innerTyping but at this point we could just use the AnyNode
anyway. I think if we decide to do it, it should be part of a follow up PR
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.
Yeah, I just wanted to raise awareness to consider that for future
Change-type: minor
77c25f0
to
79d5bce
Compare
Change-type: minor