Fix groupby resource

[jpwhite4]

See https://app.asana.com/0/342819846538629/1184363894649545

will also need updates in the Job Performance module and the XSEDE module.

[jpwhite4]

The keen-eyed developer might ask why the regression test results are unchanged. The answer is that the CI build only has one row in the specs table per resource so the multiplication factor is unity.

[plessbd]

The test you removed also tells you why it was in there in the first place like that, though not a good enough reason...

[jpwhite4]

So the code is a bit more squirelly than I originally thought. The grant type groupby uses this additional join constraint:

        "additional_join_constraints": [
            {
                "aggregate_expr": "granttype_id",
                "aggregate_table": "account",
                "attribute_expr": "id",
                "attribute_table": "granttype",
                "operation": "="
            }
        ],

which slightly misuses the code since the "aggregate_table" property is supposed to reference a table in the aggregate schema (i.e. modw_aggregates). From this snippet in the GroupBy.php file

new Table($query->getDataTable()->getSchema(), $constraint->aggregate_table, $constraint->aggregate_table)

However, the query class uses the table alias to uniquely identify tables so since a table with the 'account' alias is already added the code produces the correct SQL (referencing the correct table modw.account rather than the non-existent table modw_aggregates.account

[jpwhite4]

At this late stage I'm tempted to add some code like this:

--- a/share/classes/Realm/GroupBy.php	2020-06-25 11:19:14.082198372 -0400
+++ b/share/classes/Realm/GroupBy.php	2020-07-13 21:16:59.245296301 -0400
@@ -927,6 +927,9 @@
                     $join->name,
                     $join->name
                 ));
+		if ($this->additionalJoinConstraints === null) {
+		    break;
+		}
             }
         }

[plessbd]

fine with me, the reason I left the test in there I think was to make sure we remembered the functionality.

[jpwhite4]

Well the issue being that the test uses the open source configuration which does not include any configs that test this code pathway. Probably easiest to just add this as a test in xsede (which does use this function).

[plessbd]

and ignore that xsede doesnt have any tests...

[jpwhite4]

See also ubccr/xdmod-supremm#247