-
Notifications
You must be signed in to change notification settings - Fork 22
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
CNDB-12075: Allow UDFs within GROUP BY clause #1494
Conversation
|
❌ Build ds-cassandra-pr-gate/PR-1494 rejected by Butler58 new test failure(s) in 2 builds Found 58 new test failuresShowing only first 15 new test failures
Found 3 known test failures |
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.
This change looks good to me.
The patch ports the important functional bits from DB-672 and makes sure these are tested thoroughly. I see no reason to not approve it - remember though that I am quite untrained when it comes to judging the CC CI test results.
I have only left a minor comment related to an assertion in a test. It refers more to the tooling usage aspect than any serious functional impact, you might as well ignore it.
test/unit/org/apache/cassandra/cql3/validation/entities/UFTest.java
Outdated
Show resolved
Hide resolved
fwiw, this is a comparison between this branch and |
535671f
to
becbccb
Compare
@adelapena Is it okay with you if I merge this to |
Sure, let's merge! |
Port of DB-672.
Checklist before you submit for review
NoSpamLogger
for log lines that may appear frequently in the logs