You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metamodel.apache.org by kaspersorensen <gi...@git.apache.org> on 2015/12/04 09:53:01 UTC
[GitHub] metamodel pull request: Added new aggregate functions FIRST, LAST,...
GitHub user kaspersorensen opened a pull request:
https://github.com/apache/metamodel/pull/74
Added new aggregate functions FIRST, LAST, RANDOM
Just sharing this little idea that I had while travelling the other day ... Adding some more aggregate functions to MetaModel.
* FIRST which picks the first value
* LAST which picks the last value
* RANDOM which picks a random value
I'll admit that limited testing has been done, so maybe share any concerns here. I can add some tests soon but too busy today. I also am not sure if the JDBC module will be able to support aggregate functions that are not represented in SQL - I guess not. That might be a blocker actually, so we should maybe have some trick to do in JDBC module like we have for scalar functions.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kaspersorensen/metamodel feature/new-aggregation-functions
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metamodel/pull/74.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #74
----
commit d7214506eb6ef994c1122241ecb00fd1c4bfef89
Author: Kasper Sørensen <i....@gmail.com>
Date: 2015-12-04T08:49:46Z
Added new aggregate functions FIRST, LAST, RANDOM
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request: Added new aggregate functions FIRST, LAST,...
Posted by LosD <gi...@git.apache.org>.
Github user LosD commented on the pull request:
https://github.com/apache/metamodel/pull/74#issuecomment-162330048
Hehe, yeah. :)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request: Added new aggregate functions FIRST, LAST,...
Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on the pull request:
https://github.com/apache/metamodel/pull/74#issuecomment-162326879
Agree. This was unintentional and just muscle-memory driven. Will fix it. Maybe we can put the eclipse formatting rules somewhere so that one can make sure he is using the single-version-of-the-formatting-truth :)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request: Added new aggregate functions FIRST, LAST,...
Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on the pull request:
https://github.com/apache/metamodel/pull/74#issuecomment-162249437
Update the PR with more tests and made the JDBC module fail fast. Will update my previous PR description accordingly.
Now this is ready for a final review IMO - any takers?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request: Added new aggregate functions FIRST, LAST,...
Posted by LosD <gi...@git.apache.org>.
Github user LosD commented on the pull request:
https://github.com/apache/metamodel/pull/74#issuecomment-162326084
LGTM, but I'm really no fan of reformatting an entire file unless we touch much of the file. It makes reviews much harder than they need to be, and destroys history.
I'd suggest we keep our hands off the reformat shortcut without selecting an area around our edits.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request: Added new aggregate functions FIRST, LAST,...
Posted by LosD <gi...@git.apache.org>.
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/74#discussion_r46772077
--- Diff: jdbc/src/test/java/org/apache/metamodel/jdbc/JdbcDataContextTest.java ---
@@ -261,6 +265,20 @@ public void testWhereScalarFunction() throws Exception {
}
}
+ public void testUnsupportedAggregateFunction() throws Exception {
+ final Connection connection = getTestDbConnection();
+ final JdbcDataContext dataContext = new JdbcDataContext(connection);
+ try {
+ dataContext.query().from("customers").select(FunctionType.RANDOM, "customernumber").execute();
+ fail("Exception expected");
+ } catch (MetaModelException e) {
+ assertEquals(
+ "Aggregate function 'RANDOM' is not supported on this JDBC database. Query rejected: "
+ + "SELECT RANDOM(\"CUSTOMERS\".\"CUSTOMERNUMBER\") FROM PUBLIC.\"CUSTOMERS\"",
+ e.getMessage());
+ }
+ }
+
--- End diff --
This is fine, though reformatting the whole file for adding a single function is a pretty history-destructive.
That much of the formatting is pretty weird doesn't make it better :)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request: Added new aggregate functions FIRST, LAST,...
Posted by LosD <gi...@git.apache.org>.
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/74#discussion_r46772088
--- Diff: jdbc/src/test/java/org/apache/metamodel/jdbc/JdbcDataContextTest.java ---
@@ -148,20 +148,22 @@ public void testExecuteQuery() throws Exception {
assertEquals(
"SELECT a._CUSTOMERNUMBER_, a._CUSTOMERNAME_, a._CONTACTLASTNAME_, a._CONTACTFIRSTNAME_, a._PHONE_, "
+ "a._ADDRESSLINE1_, a._ADDRESSLINE2_, a._CITY_, a._STATE_, a._POSTALCODE_, a._COUNTRY_, "
- + "a._SALESREPEMPLOYEENUMBER_, a._CREDITLIMIT_ FROM PUBLIC._CUSTOMERS_ a", q.toString()
- .replace('\"', '_'));
+ + "a._SALESREPEMPLOYEENUMBER_, a._CREDITLIMIT_ FROM PUBLIC._CUSTOMERS_ a",
+ q.toString().replace('\"', '_'));
DataSet result = strategy.executeQuery(q);
assertTrue(result.next());
assertEquals(
"Row[values=[103, Atelier graphique, Schmitt, Carine, 40.32.2555, 54, rue Royale, null, Nantes, null, "
- + "44000, France, 1370, 21000.0]]", result.getRow().toString());
+ + "44000, France, 1370, 21000.0]]",
+ result.getRow().toString());
assertTrue(result.next());
assertTrue(result.next());
assertTrue(result.next());
assertTrue(result.next());
assertEquals(
"Row[values=[121, Baane Mini Imports, Bergulfsen, Jonas, 07-98 9555, Erling Skakkes gate 78, null, "
- + "Stavern, null, 4110, Norway, 1504, 81700.0]]", result.getRow().toString());
+ + "Stavern, null, 4110, Norway, 1504, 81700.0]]",
+ result.getRow().toString());
--- End diff --
Example of weird reformat
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request: Added new aggregate functions FIRST, LAST,...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/metamodel/pull/74
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---