-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Apply DomainTranslator to STARTS_WITH function #4669
Conversation
1c99fb2
to
40f8f89
Compare
if (args.size() != 2) { | ||
return Optional.empty(); | ||
} | ||
if (!(args.get(0) instanceof SymbolReference)) { |
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.
please extarct args.get(0)
and args.get(1)
as separate variables
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.
fixed.
presto-main/src/test/java/io/prestosql/sql/planner/TestDomainTranslator.java
Show resolved
Hide resolved
40f8f89
to
aaddb47
Compare
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.
lgtm % one extra test case
presto-main/src/test/java/io/prestosql/sql/planner/TestDomainTranslator.java
Show resolved
Hide resolved
assertUnsupportedPredicate(startsWith(C_VARCHAR, stringLiteral(""))); | ||
|
||
// non-ASCII | ||
testSimpleComparison( |
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.
Could you add test for complement
case?
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.
Added.
@@ -1582,6 +1582,39 @@ public void testLikePredicate() | |||
assertUnsupportedPredicate(not(like(C_VARCHAR, stringLiteral("abc\\_def")))); | |||
} | |||
|
|||
@Test | |||
public void testStartsWithFunction() |
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.
Could you add a test for generic function call (that it's not supported)?
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.
Added testUnsupportedFunctions()
which tests some unsupported functions.
aaddb47
to
990129e
Compare
merged, thanks! |
No description provided.