Flatten domain sort. Fixes #3727

src/compile/data/formatparse.ts

@@ -1,12 +1,12 @@
-import {isString} from 'util';
-import {isNumber, toSet} from 'vega-util';
+import {isNumber, isString, toSet} from 'vega-util';
 import {AncestorParse} from '.';
 import {isCountingAggregateOp} from '../../aggregate';
 import {DateTime, isDateTime} from '../../datetime';
-import {isNumberFieldDef, isTimeFieldDef} from '../../fielddef';
+import {isNumberFieldDef, isScaleFieldDef, isTimeFieldDef} from '../../fielddef';
 import * as log from '../../log';
 import {forEachLeaf} from '../../logical';
 import {isFieldEqualPredicate, isFieldOneOfPredicate, isFieldPredicate, isFieldRangePredicate} from '../../predicate';
+import {isSortField} from '../../sort';
 import {FilterTransform} from '../../transform';
 import {accessPathDepth, accessPathWithDatum, Dict, duplicate, keys, removePathFromField, StringSet} from '../../util';
 import {VgFormulaTransform} from '../../vega.schema';
@@ -122,14 +122,20 @@ export class ParseNode extends DataFlowNode {
         if (isTimeFieldDef(fieldDef)) {
           implicit[fieldDef.field] = 'date';
         } else if (isNumberFieldDef(fieldDef)) {
-          if (isCountingAggregateOp(fieldDef.aggregate)) {
-            return;
+          if (!isCountingAggregateOp(fieldDef.aggregate)) {
+            implicit[fieldDef.field] = 'number';
           }
-          implicit[fieldDef.field] = 'number';
         } else if (accessPathDepth(fieldDef.field) > 1) {
           // For non-date/non-number (strings and booleans), derive a flattened field for a referenced nested field.
           // (Parsing numbers / dates already flattens numeric and temporal fields.)
-          implicit[fieldDef.field] = 'flatten';
+          if (!(fieldDef.field in implicit)) {
+            implicit[fieldDef.field] = 'flatten';
+          }
+        } else if (isScaleFieldDef(fieldDef) && isSortField(fieldDef.sort) && accessPathDepth(fieldDef.sort.field) > 1) {
+          // Flatten fields that we sort by but that are not otherwise flattened.
+          if (!(fieldDef.sort.field in implicit)) {
+            implicit[fieldDef.sort.field] = 'flatten';
+          }
         }
       });
     }
@@ -141,12 +147,12 @@ export class ParseNode extends DataFlowNode {
    * Creates a parse node from "explicit" parse and "implicit" parse and updates ancestorParse.
    */
   private static makeWithAncestors(parent: DataFlowNode, explicit: Dict<string>, implicit: Dict<string>, ancestorParse: AncestorParse) {
-    // We should not parse what has already been parsed in a parent (explicitly or implicitly) or what has been derived (maked as "derived").
+    // We should not parse what has already been parsed in a parent (explicitly or implicitly) or what has been derived (maked as "derived"). We also don't need to flatten a field that has already been parsed.
     for (const field of keys(implicit)) {
       const parsedAs = ancestorParse.getWithExplicit(field);
       if (parsedAs.value !== undefined) {
         // We always ignore derived fields even if they are implicitly defined because we expect users to create the right types.
-        if (parsedAs.explicit || parsedAs.value === implicit[field] || parsedAs.value === 'derived') {
+        if (parsedAs.explicit || parsedAs.value === implicit[field] || parsedAs.value === 'derived' || implicit[field] === 'flatten') {
           delete implicit[field];
         } else {
           log.warn(log.message.differentParse(field, implicit[field], parsedAs.value));

src/compile/scale/domain.ts

@@ -7,7 +7,7 @@ import {DateTime, dateTimeExpr, isDateTime} from '../../datetime';
 import {FieldDef, ScaleFieldDef} from '../../fielddef';
 import * as log from '../../log';
 import {Domain, hasDiscreteDomain, isBinScale, isSelectionDomain, ScaleConfig, ScaleType} from '../../scale';
-import {EncodingSortField, isSortArray, isSortField} from '../../sort';
+import {EncodingSortField, isSortArray, isSortField, SortField} from '../../sort';
 import {hash} from '../../util';
 import * as util from '../../util';
 import {isDataRefUnionedDomain, isFieldRefUnionDomain} from '../../vega.schema';
@@ -57,7 +57,7 @@ function parseUnitScaleDomain(model: UnitModel) {
 
       // FIXME: replace this with a special property in the scaleComponent
       localScaleCmpt.set('domainRaw', {
-        signal: SELECTION_DOMAIN + hash(specifiedDomain)
+        signal: SELECTION_DOMAIN + util.hash(specifiedDomain)
       }, true);
     }
 
@@ -288,7 +288,11 @@ export function domainSort(model: UnitModel, channel: ScaleChannel, scaleType: S
 
   // Sorted based on an aggregate calculation over a specified sort field (only for ordinal scale)
   if (isSortField(sort)) {
-    return sort;
+    // flatten nested fields
+    return {
+      ...sort,
+      ...(sort.field ? {field: util.replacePathInField(sort.field)} : {})
+    };
   }
 
   if (sort === 'descending') {

src/fielddef.ts

@@ -283,7 +283,7 @@ export function isValueDef<F>(channelDef: ChannelDef<F>): channelDef is ValueDef
   return channelDef && 'value' in channelDef && channelDef['value'] !== undefined;
 }
 
-export function isScaleFieldDef(channelDef: ChannelDef<any>): channelDef is ScaleFieldDef<any> {
+export function isScaleFieldDef<F>(channelDef: ChannelDef<F>): channelDef is ScaleFieldDef<F> {
   return !!channelDef && (!!channelDef['scale'] || !!channelDef['sort']);
 }
 

src/util.ts

@@ -328,5 +328,8 @@ export function removePathFromField(path: string) {
  * Count the depth of the path. Returns 1 for fields that are not nested.
  */
 export function accessPathDepth(path: string) {
+  if (!path) {
+    return 0;
+  }
   return splitAccessPath(path).length;
 }

test/compile/data/formatparse.test.ts

@@ -24,6 +24,19 @@ describe('compile/data/formatparse', () => {
       });
     });
 
+    it('should flatten nested fields that are used to sort domains', () => {
+      const model = parseUnitModel({
+        "mark": "point",
+        "encoding": {
+          x: {field: 'a', type: 'ordinal', sort: {field: 'foo.bar', op: 'mean'}},
+        }
+      });
+
+      assert.deepEqual(ParseNode.makeImplicitFromEncoding(null, model, new AncestorParse()).parse, {
+        'foo.bar': 'flatten'
+      });
+    });
+
     it('should return a correct customized parse.', () => {
       const model = parseUnitModel({
         "data": {"url": "a.json", "format": {"parse": {"c": "number", "d": "date"}}},

test/compile/scale/domain.test.ts

@@ -837,5 +837,15 @@ describe('compile/scale', () => {
       });
       assert.deepEqual<VgSortField>(domainSort(model, 'x', ScaleType.ORDINAL), {op: 'min', field: 'x_a_sort_index', order: 'ascending'});
     });
+
+    it('should return sort with flattened field access', () => {
+      const model = parseUnitModel({
+        mark: 'bar',
+        encoding: {
+          x: {field: 'a', type: 'ordinal', sort: {field: 'foo.bar', op: 'mean'}},
+        }
+      });
+      assert.deepEqual<VgSortField>(domainSort(model, 'x', ScaleType.ORDINAL), {op: 'mean', field: 'foo\\.bar'});
+    });
   });
 });