ENT-1428 Vault queries - change SQL generation for aggregate function in 'group by' clause. (#426)

Vault queries: 'order by' function aggregation e.g. 'order by sum(col)' is replaced by the position of the selected column (e.g. 'order by 1').
Rationale: NonStop database doesn't support aggregation function in order by clause e.g. 'order by (sum col_x)'. I couldn't find a way to force Hibernate to produce alias e.g. ' select sum(col_x) as alias_x... order by also_x ...',   and the only solution supported by all db an HPE NonStop is to use positional parameter e.g. 'select sum(col_x) as alias_x... order by 1 ...'

node/src/main/kotlin/net/corda/node/services/vault/HibernateQueryCriteriaParser.kt

@@ -299,29 +299,36 @@ class HibernateQueryCriteriaParser(val contractStateType: Class<out ContractStat
                         }
                 //TODO investigate possibility to avoid producing redundant joins in SQL for multiple aggregate functions against the same table
                 aggregateExpressions.add(aggregateExpression)
-                // optionally order by this aggregate function
+                // Some databases may not support aggregate expression in 'group by' clause e.g. 'group by sum(col)',
-                expression.orderBy?.let {
+                // Hibernate Criteria Builder can't produce alias 'group by col_alias', and the only solution is to use a positional parameter 'group by 1'
-                    val orderCriteria =
+                val orderByColumnPosition = aggregateExpressions.size
-                            when (expression.orderBy!!) {
+                var shiftLeft = 0
-                                Sort.Direction.ASC -> criteriaBuilder.asc(aggregateExpression)
-                                Sort.Direction.DESC -> criteriaBuilder.desc(aggregateExpression)
-                            }
-                    criteriaQuery.orderBy(orderCriteria)
-                }
                 // add optional group by clauses
                 expression.groupByColumns?.let { columns ->
                     val groupByExpressions =
                             columns.map { _column ->
                                 val path = root.get<Any?>(getColumnName(_column))
+                                val columnNumberBeforeRemoval = aggregateExpressions.size
                                 if (path is SingularAttributePath) //remove the same columns from different joins to match the single column in 'group by' only (from the last join)
                                     aggregateExpressions.removeAll {
                                         elem -> if (elem is SingularAttributePath) elem.attribute.javaMember == path.attribute.javaMember else false
                                     }
+                                shiftLeft += columnNumberBeforeRemoval - aggregateExpressions.size //record how many times a duplicated column was removed (from the previous 'parseAggregateFunction' run)
                                 aggregateExpressions.add(path)
                                 path
                             }
                     criteriaQuery.groupBy(groupByExpressions)
                 }
+                // optionally order by this aggregate function
+                expression.orderBy?.let {
+                    val orderCriteria =
+                            when (expression.orderBy!!) {
+                                // when adding column position of 'group by' shift in case columns were removed
+                                Sort.Direction.ASC -> criteriaBuilder.asc(criteriaBuilder.literal<Int>(orderByColumnPosition - shiftLeft))
+                                Sort.Direction.DESC -> criteriaBuilder.desc(criteriaBuilder.literal<Int>(orderByColumnPosition - shiftLeft))
+                            }
+                    criteriaQuery.orderBy(orderCriteria)
+                }
                 return aggregateExpression
             }
             else -> throw VaultQueryException("Not expecting $columnPredicate")

node/src/test/kotlin/net/corda/node/services/vault/VaultQueryTests.kt

@@ -767,6 +767,72 @@ open class VaultQueryTests {
         }
     }
 
+    @Test
+    fun `aggregate functions with single group clause desc first column`() {
+        database.transaction {
+            listOf(100.DOLLARS, 200.DOLLARS, 300.DOLLARS, 400.POUNDS, 500.SWISS_FRANCS).zip(1..5).forEach { (howMuch, states) ->
+                vaultFiller.fillWithSomeTestCash(howMuch, notaryServices, states, DUMMY_CASH_ISSUER)
+            }
+            val sum = builder { CashSchemaV1.PersistentCashState::pennies.sum(groupByColumns = listOf(CashSchemaV1.PersistentCashState::currency), orderBy = Sort.Direction.DESC) }
+            val max = builder { CashSchemaV1.PersistentCashState::pennies.max(groupByColumns = listOf(CashSchemaV1.PersistentCashState::currency)) }
+            val min = builder { CashSchemaV1.PersistentCashState::pennies.min(groupByColumns = listOf(CashSchemaV1.PersistentCashState::currency)) }
+
+            val results = vaultService.queryBy<FungibleAsset<*>>(VaultCustomQueryCriteria(sum)
+                    .and(VaultCustomQueryCriteria(max))
+                    .and(VaultCustomQueryCriteria(min)))
+
+            assertThat(results.otherResults).hasSize(12)
+
+            assertThat(results.otherResults.subList(0,4)).isEqualTo(listOf(60000L, 11298L, 8702L, "USD"))
+            assertThat(results.otherResults.subList(4,8)).isEqualTo(listOf(50000L, 10274L, 9481L, "CHF"))
+            assertThat(results.otherResults.subList(8,12)).isEqualTo(listOf(40000L, 10343L, 9351L, "GBP"))
+        }
+    }
+
+    @Test
+    fun `aggregate functions with single group clause desc mid column`() {
+        database.transaction {
+            listOf(100.DOLLARS, 200.DOLLARS, 300.DOLLARS, 400.POUNDS, 500.SWISS_FRANCS).zip(1..5).forEach { (howMuch, states) ->
+                vaultFiller.fillWithSomeTestCash(howMuch, notaryServices, states, DUMMY_CASH_ISSUER)
+            }
+            val sum = builder { CashSchemaV1.PersistentCashState::pennies.sum(groupByColumns = listOf(CashSchemaV1.PersistentCashState::currency)) }
+            val max = builder { CashSchemaV1.PersistentCashState::pennies.max(groupByColumns = listOf(CashSchemaV1.PersistentCashState::currency), orderBy = Sort.Direction.DESC) }
+            val min = builder { CashSchemaV1.PersistentCashState::pennies.min(groupByColumns = listOf(CashSchemaV1.PersistentCashState::currency)) }
+
+            val results = vaultService.queryBy<FungibleAsset<*>>(VaultCustomQueryCriteria(sum)
+                    .and(VaultCustomQueryCriteria(max))
+                    .and(VaultCustomQueryCriteria(min)))
+
+            assertThat(results.otherResults).hasSize(12)
+
+            assertThat(results.otherResults.subList(0,4)).isEqualTo(listOf(60000L, 11298L, 8702L, "USD"))
+            assertThat(results.otherResults.subList(4,8)).isEqualTo(listOf(40000L, 10343L, 9351L, "GBP"))
+            assertThat(results.otherResults.subList(8,12)).isEqualTo(listOf(50000L, 10274L, 9481L, "CHF"))
+        }
+    }
+
+    @Test
+    fun `aggregate functions with single group clause desc last column`() {
+        database.transaction {
+            listOf(100.DOLLARS, 200.DOLLARS, 300.DOLLARS, 400.POUNDS, 500.SWISS_FRANCS).zip(1..5).forEach { (howMuch, states) ->
+                vaultFiller.fillWithSomeTestCash(howMuch, notaryServices, states, DUMMY_CASH_ISSUER)
+            }
+            val sum = builder { CashSchemaV1.PersistentCashState::pennies.sum(groupByColumns = listOf(CashSchemaV1.PersistentCashState::currency)) }
+            val max = builder { CashSchemaV1.PersistentCashState::pennies.max(groupByColumns = listOf(CashSchemaV1.PersistentCashState::currency)) }
+            val min = builder { CashSchemaV1.PersistentCashState::pennies.min(groupByColumns = listOf(CashSchemaV1.PersistentCashState::currency), orderBy = Sort.Direction.DESC) }
+
+            val results = vaultService.queryBy<FungibleAsset<*>>(VaultCustomQueryCriteria(sum)
+                    .and(VaultCustomQueryCriteria(max))
+                    .and(VaultCustomQueryCriteria(min)))
+
+            assertThat(results.otherResults).hasSize(12)
+
+            assertThat(results.otherResults.subList(0,4)).isEqualTo(listOf(50000L, 10274L, 9481L, "CHF"))
+            assertThat(results.otherResults.subList(4,8)).isEqualTo(listOf(40000L, 10343L, 9351L, "GBP"))
+            assertThat(results.otherResults.subList(8,12)).isEqualTo(listOf(60000L, 11298L, 8702L, "USD"))
+        }
+    }
+
     @Test
     fun `aggregate functions sum by issuer and currency and sort by aggregate sum`() {
         database.transaction {