[GitHub] flink issue #2088: [FLINK-3859] [table] Add BigDecimal/BigInteger support to...
Github user twalthr commented on the issue: https://github.com/apache/flink/pull/2088 Thanks Fabian. Merging... --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2088: [FLINK-3859] [table] Add BigDecimal/BigInteger support to...
Github user fhueske commented on the issue: https://github.com/apache/flink/pull/2088 Thanks for refactoring the tests @twalthr! LGTM --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2088: [FLINK-3859] [table] Add BigDecimal/BigInteger support to...
Github user twalthr commented on the issue: https://github.com/apache/flink/pull/2088 If there are no objections, I would merge this later today... --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2088: [FLINK-3859] [table] Add BigDecimal/BigInteger support to...
Github user twalthr commented on the issue: https://github.com/apache/flink/pull/2088 @fhueske I have changed the `ExpressionTestBase`. Now one unit tests only needs one compilation step. The runtime is still not perfect but could be reduced by about 75 % (at least on my PC). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2088: [FLINK-3859] [table] Add BigDecimal/BigInteger support to...
Github user twalthr commented on the issue: https://github.com/apache/flink/pull/2088 I also already recognized the problem of long running tests. I think will rework the test base again. So that there is one compilation per unit test. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2088: [FLINK-3859] [table] Add BigDecimal/BigInteger support to...
Github user fhueske commented on the issue: https://github.com/apache/flink/pull/2088 Won't merge this PR at this point. A few Travis builds timed out and I found that the added expression tests increase build time by about one minute due to the code-gen compilation overhead. @twalthr, do you think we can adapt the tests to batch several expressions into a single class with multiple methods to invoke the compiler less frequently? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2088: [FLINK-3859] [table] Add BigDecimal/BigInteger support to...
Github user fhueske commented on the issue: https://github.com/apache/flink/pull/2088 Merging --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2088: [FLINK-3859] [table] Add BigDecimal/BigInteger support to...
Github user fhueske commented on the issue: https://github.com/apache/flink/pull/2088 Thanks for the update @twalthr. The changes look good. Should be good to merge after the aggregators commit is added. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2088: [FLINK-3859] [table] Add BigDecimal/BigInteger support to...
Github user twalthr commented on the issue: https://github.com/apache/flink/pull/2088 @fhueske I have updated my PR. Now comparisons should work. Once the build succeeded, I will add your commits. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2088: [FLINK-3859] [table] Add BigDecimal/BigInteger support to...
Github user fhueske commented on the issue: https://github.com/apache/flink/pull/2088 Hi @twalthr, I implemented the aggregation functions and pushed the commit to a branch in my repository: https://github.com/fhueske/flink/tree/tableDecimal I tried to open a PR against your repository, but Github didn't offer it as an option. :-( --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2088: [FLINK-3859] [table] Add BigDecimal/BigInteger support to...
Github user fhueske commented on the issue: https://github.com/apache/flink/pull/2088 Think we also need aggregation functions for `DECIMAL` otherwise this won't work: ``` SELECT sum(myInt * 1.23) FROM MyTable ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2088: [FLINK-3859] [table] Add BigDecimal/BigInteger support to...
Github user fhueske commented on the issue: https://github.com/apache/flink/pull/2088 Thanks for the PR, @twalthr! Looks mostly good. I found a few cases that needs to be fix. We also should think about how to handle SQL `DECIMAL` types with fixed precision / scale, e.g., how to handle something like `SELECT myDouble AS DECIMAL(4,2)`. Do you think this could be easily added with this PR or rather be fix in a later issue? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
