Throw on duplicate alias (#9343)

Author: MarkDuckworth

packages/firestore/src/util/pipeline_util.ts

@@ -15,7 +15,7 @@
  * limitations under the License.
  */
 
-import { vector } from '../api';
+import { FirestoreError, vector } from '../api';
 import {
   _constant,
   AggregateFunction,
@@ -31,6 +31,7 @@ import {
 } from '../lite-api/expressions';
 import { VectorValue } from '../lite-api/vector_value';
 
+import { fail } from './assert';
 import { isPlainObject } from './input_validation';
 import { isFirestoreValue } from './proto';
 import { isString } from './types';
@@ -40,13 +41,29 @@ export function selectablesToMap(
 ): Map<string, Expression> {
   const result = new Map<string, Expression>();
   for (const selectable of selectables) {
+    let alias: string;
+    let expression: Expression;
     if (typeof selectable === 'string') {
-      result.set(selectable as string, field(selectable));
+      alias = selectable as string;
+      expression = field(selectable);
     } else if (selectable instanceof Field) {
-      result.set(selectable.alias, selectable.expr);
+      alias = selectable.alias;
+      expression = selectable.expr;
     } else if (selectable instanceof AliasedExpression) {
-      result.set(selectable.alias, selectable.expr);
+      alias = selectable.alias;
+      expression = selectable.expr;
+    } else {
+      fail(0x5319, '`selectable` has an unsupported type', { selectable });
     }
+
+    if (result.get(alias) !== undefined) {
+      throw new FirestoreError(
+        'invalid-argument',
+        `Duplicate alias or field '${alias}'`
+      );
+    }
+
+    result.set(alias, expression);
   }
   return result;
 }
@@ -56,6 +73,13 @@ export function aliasedAggregateToMap(
 ): Map<string, AggregateFunction> {
   return aliasedAggregatees.reduce(
     (map: Map<string, AggregateFunction>, selectable: AliasedAggregate) => {
+      if (map.get(selectable.alias) !== undefined) {
+        throw new FirestoreError(
+          'invalid-argument',
+          `Duplicate alias or field '${selectable.alias}'`
+        );
+      }
+
       map.set(selectable.alias, selectable.aggregate as AggregateFunction);
       return map;
     },

packages/firestore/test/integration/api/pipeline.test.ts

@@ -879,6 +879,27 @@ const timestampDeltaMS = 1000;
         });
       });
 
+      it('throws on Duplicate aliases', async () => {
+        expect(() =>
+          firestore
+            .pipeline()
+            .collection(randomCol.path)
+            .aggregate(countAll().as('count'), count('foo').as('count'))
+        ).to.throw("Duplicate alias or field 'count'");
+      });
+
+      it('throws on duplicate group aliases', async () => {
+        expect(() =>
+          firestore
+            .pipeline()
+            .collection(randomCol.path)
+            .aggregate({
+              accumulators: [countAll().as('count')],
+              groups: ['bax', field('bar').as('bax')]
+            })
+        ).to.throw("Duplicate alias or field 'bax'");
+      });
+
       it('supports aggregate options', async () => {
         let snapshot = await execute(
           firestore
@@ -1081,6 +1102,16 @@ const timestampDeltaMS = 1000;
         );
       });
 
+      it('throws on Duplicate aliases', async () => {
+        expect(() => {
+          firestore
+            .pipeline()
+            .collection(randomCol.path)
+            .limit(1)
+            .select(constant(1).as('foo'), constant(2).as('foo'));
+        }).to.throw("Duplicate alias or field 'foo'");
+      });
+
       it('supports options', async () => {
         const snapshot = await execute(
           firestore
@@ -1154,6 +1185,17 @@ const timestampDeltaMS = 1000;
         );
       });
 
+      it('throws on Duplicate aliases', async () => {
+        expect(() =>
+          firestore
+            .pipeline()
+            .collection(randomCol.path)
+            .select('title', 'author')
+            .addFields(constant('bar').as('foo'), constant('baz').as('foo'))
+            .sort(field('author').ascending())
+        ).to.throw("Duplicate alias or field 'foo'");
+      });
+
       it('supports options', async () => {
         const snapshot = await execute(
           firestore