Merge pull request #28 from microsoft/fixers

Fixers

Author: niclasko

flowquery-py/src/parsing/operations/group_by.py

@@ -114,6 +114,9 @@ def generate_results(
         if node is None:
             node = self._root
 
+        if mapper_index == 0 and len(node.children) == 0 and len(self.mappers) > 0:
+            return
+
         if len(node.children) > 0:
             for child in node.children.values():
                 self.mappers[mapper_index].overridden = child.value

flowquery-py/tests/compute/test_runner.py

@@ -378,6 +378,21 @@ async def test_aggregated_with_and_return(self):
         assert results[0] == {"i": 1, "sum": 12}
         assert results[1] == {"i": 2, "sum": 12}
 
+    @pytest.mark.asyncio
+    async def test_aggregated_with_on_empty_result_set(self):
+        """Test aggregated with on empty result set does not crash."""
+        runner = Runner(
+            """
+            unwind [] as i
+            unwind [1, 2] as j
+            with i, count(j) as cnt
+            return i, cnt
+            """
+        )
+        await runner.run()
+        results = runner.results
+        assert len(results) == 0
+
     @pytest.mark.asyncio
     async def test_aggregated_with_using_collect_and_return(self):
         """Test aggregated with using collect and return."""

src/parsing/operations/group_by.ts

@@ -50,10 +50,10 @@ class GroupBy extends Projection {
     }
     private map() {
         let node: Node = this.current;
-        for(const mapper of this.mappers) {
+        for (const mapper of this.mappers) {
             const value: any = mapper.value();
             let child: Node | undefined = node.children.get(value);
-            if(child === undefined) {
+            if (child === undefined) {
                 child = new Node(value);
                 node.children.set(value, child);
             }
@@ -62,38 +62,46 @@ class GroupBy extends Projection {
         this.current = node;
     }
     private reduce() {
-        if(this.current.elements === null) {
-            this.current.elements = this.reducers.map(reducer => reducer.element());
+        if (this.current.elements === null) {
+            this.current.elements = this.reducers.map((reducer) => reducer.element());
         }
         const elements: AggregationElement[] = this.current.elements;
         this.reducers.forEach((reducer, index) => {
             reducer.reduce(elements[index]);
         });
     }
     private get mappers(): Expression[] {
-        if(this._mappers === null) {
+        if (this._mappers === null) {
             this._mappers = [...this._generate_mappers()];
         }
         return this._mappers;
     }
     private *_generate_mappers(): Generator<Expression> {
-        for(const [expression, _] of this.expressions()) {
-            if(expression.mappable()) {
+        for (const [expression, _] of this.expressions()) {
+            if (expression.mappable()) {
                 yield expression;
             }
         }
     }
     private get reducers(): AggregateFunction[] {
-        if(this._reducers === null) {
-            this._reducers = this.children.map((child) => {
-                return (child as Expression).reducers();
-            }).flat();
+        if (this._reducers === null) {
+            this._reducers = this.children
+                .map((child) => {
+                    return (child as Expression).reducers();
+                })
+                .flat();
         }
         return this._reducers;
     }
-    public *generate_results(mapperIndex: number = 0, node: Node = this.root): Generator<Record<string, any>> {
-        if(node.children.size > 0) {
-            for(const child of node.children.values()) {
+    public *generate_results(
+        mapperIndex: number = 0,
+        node: Node = this.root
+    ): Generator<Record<string, any>> {
+        if (mapperIndex === 0 && node.children.size === 0 && this.mappers.length > 0) {
+            return;
+        }
+        if (node.children.size > 0) {
+            for (const child of node.children.values()) {
                 this.mappers[mapperIndex].overridden = child.value;
                 yield* this.generate_results(mapperIndex + 1, child);
             }
@@ -102,10 +110,10 @@ class GroupBy extends Projection {
                 this.reducers[reducerIndex].overridden = element.value;
             });
             const record: Record<string, any> = {};
-            for(const [expression, alias] of this.expressions()) {
+            for (const [expression, alias] of this.expressions()) {
                 record[alias] = expression.value();
             }
-            if(this.where) {
+            if (this.where) {
                 yield record;
             }
         }
@@ -114,11 +122,11 @@ class GroupBy extends Projection {
         this._where = where;
     }
     public get where(): boolean {
-        if(this._where === null) {
+        if (this._where === null) {
             return true;
         }
         return this._where.value();
     }
-};
+}
 
-export default GroupBy;
+export default GroupBy;

tests/compute/runner.test.ts

@@ -334,6 +334,20 @@ test("Test aggregated with and return", async () => {
     expect(results[1]).toEqual({ i: 2, sum: 12 });
 });
 
+test("Test aggregated with on empty result set", async () => {
+    const runner = new Runner(
+        `
+        unwind [] as i
+        unwind [1, 2] as j
+        with i, count(j) as cnt
+        return i, cnt
+        `
+    );
+    await runner.run();
+    const results = runner.results;
+    expect(results.length).toBe(0);
+});
+
 test("Test aggregated with using collect and return", async () => {
     const runner = new Runner(
         `