You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/02/20 12:56:54 UTC

[GitHub] [ignite] korlov42 commented on a change in pull request #8815: IGNITE-14157: Return good names for columns related to aggregates

korlov42 commented on a change in pull request #8815:
URL: https://github.com/apache/ignite/pull/8815#discussion_r579650947



##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/QueryChecker.java
##########
@@ -303,6 +316,13 @@ public void check() {
 
         FieldsQueryCursor<List<?>> cur = cursors.get(0);
 
+        if(expectedColumnNames != null) {
+            assertEquals("numer of columns are different", expectedColumnNames.size(), cur.getColumnsCount());

Review comment:
       ```suggestion
               assertEquals("number of columns are different", expectedColumnNames.size(), cur.getColumnsCount());
   ```

##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/AggregatesIntegrationTest.java
##########
@@ -102,6 +108,21 @@ public void countOfNonNumericField() throws InterruptedException {
             .check();
     }
 
+    private void createAndPopulateTable() {

Review comment:
       missing javadoc

##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/QueryChecker.java
##########
@@ -264,6 +267,16 @@ public QueryChecker returns(Object... res) {
         return CoreMatchers.containsString("IgniteUnionAll(all=[" + all + "])");
     }
 
+    /** */
+    public QueryChecker columnNames(String... columns) {

Review comment:
       I would prefer to override expected column names each time this method invoked. I can hardly imagine using it like this:
   ```
   assertQuery("select c1, c2, c3, c4 from my_table")
       .columnNames("c1", "c2")
       .columnNames("c3", "c4")
       .check()
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org