Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/1926#issuecomment-213565509
Thanks for opening this PR @dawidwys!
I skimmed over the changes and it looks good. I will do a more detailed
review hopefully soon.
Thanks also for adding a lot of tests. It is definitely good to have
extensive test coverage, but end-to-end tests such as yours add quite a bit to
the build time. Flink uses Travis as CI service which kills builds after 2h.
Unfortunately, we are experiencing build time outs already and have to be
careful when adding tests.
I would like to ask you to remove some of the tests which check for
different expression syntax but end up in identical executions. In addition, it
would be good to add one SQL test that executes a query which sorts on two
fields to have the SQL part covered.
We will add a unit test framework, that checks for correct parsing of the
expressions without actually executing queries in a separate effort.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
