[Lens] Formula: better messages for unsupported field types

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/editor/formula_editor.tsx

@@ -49,6 +49,18 @@ import { regenerateLayerFromAst } from '../parse';
 import { filterByVisibleOperation } from '../util';
 import { getColumnTimeShiftWarnings, getDateHistogramInterval } from '../../../../time_shift_utils';
 
+function tableHasData(
+  activeData: ParamEditorProps<FormulaIndexPatternColumn>['activeData'],
+  layerId: string,
+  columnId: string
+) {
+  const table = activeData?.[layerId];
+  if (!table || table.rows.length === 0) {
+    return false;
+  }
+  return table.rows.some((row) => row[columnId] != null);
+}
+
 export const WrappedFormulaEditor = ({
   activeData,
   ...rest
@@ -59,7 +71,13 @@ export const WrappedFormulaEditor = ({
     activeData,
     rest.layerId
   );
-  return <MemoizedFormulaEditor {...rest} dateHistogramInterval={dateHistogramInterval} />;
+  return (
+    <MemoizedFormulaEditor
+      {...rest}
+      dateHistogramInterval={dateHistogramInterval}
+      hasData={tableHasData(activeData, rest.layerId, rest.columnId)}
+    />
+  );
 };
 
 const MemoizedFormulaEditor = React.memo(FormulaEditor);
@@ -76,8 +94,10 @@ export function FormulaEditor({
   isFullscreen,
   setIsCloseable,
   dateHistogramInterval,
+  hasData,
 }: Omit<ParamEditorProps<FormulaIndexPatternColumn>, 'activeData'> & {
   dateHistogramInterval: ReturnType<typeof getDateHistogramInterval>;
+  hasData: boolean;
 }) {
   const [text, setText] = useState(currentColumn.params.formula);
   const [warnings, setWarnings] = useState<
@@ -180,7 +200,12 @@ export function FormulaEditor({
       }
 
       if (errors.length) {
-        if (currentColumn.params.isFormulaBroken) {
+        // Replace the previous error with the new one
+        const previousFormulaWasBroken = currentColumn.params.isFormulaBroken;
+        // If the user is changing a previous formula and there are currently no result
+        // show the most up-to-date state with the error message.
+        const previousFormulaWasOkButNoData = !currentColumn.params.isFormulaBroken && !hasData;
+        if (previousFormulaWasBroken || previousFormulaWasOkButNoData) {
           // If the formula is already broken, show the latest error message in the workspace
           if (currentColumn.params.formula !== text) {
             updateLayer(

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx

@@ -33,11 +33,27 @@ const operationDefinitionMap: Record<string, GenericOperationDefinition> = {
       scale: 'ratio',
       timeScale: false,
     }),
+    getPossibleOperationForField: () => ({ scale: 'ratio' }),
+  } as unknown as GenericOperationDefinition,
+  terms: {
+    input: 'field',
+    getPossibleOperationForField: () => ({ scale: 'ordinal' }),
+  } as unknown as GenericOperationDefinition,
+  sum: {
+    input: 'field',
+    filterable: true,
+    getPossibleOperationForField: () => ({ scale: 'ratio' }),
+  } as unknown as GenericOperationDefinition,
+  last_value: {
+    input: 'field',
+    getPossibleOperationForField: ({ type }) => ({
+      scale: type === 'string' ? 'ordinal' : 'ratio',
+    }),
+  } as GenericOperationDefinition,
+  max: {
+    input: 'field',
+    getPossibleOperationForField: () => ({ scale: 'ratio' }),
   } as unknown as GenericOperationDefinition,
-  terms: { input: 'field' } as GenericOperationDefinition,
-  sum: { input: 'field', filterable: true } as GenericOperationDefinition,
-  last_value: { input: 'field' } as GenericOperationDefinition,
-  max: { input: 'field' } as GenericOperationDefinition,
   count: {
     input: 'field',
     filterable: true,
@@ -50,8 +66,12 @@ const operationDefinitionMap: Record<string, GenericOperationDefinition> = {
       scale: 'ratio',
       timeScale: false,
     }),
+    getPossibleOperationForField: () => ({ scale: 'ratio' }),
+  } as unknown as GenericOperationDefinition,
+  derivative: {
+    input: 'fullReference',
+    getPossibleOperationForField: () => ({ scale: 'ratio' }),
   } as unknown as GenericOperationDefinition,
-  derivative: { input: 'fullReference' } as GenericOperationDefinition,
   moving_average: {
     input: 'fullReference',
     operationParams: [{ name: 'window', type: 'number', required: true }],
@@ -66,8 +86,12 @@ const operationDefinitionMap: Record<string, GenericOperationDefinition> = {
       references,
     }),
     getErrorMessage: () => ['mock error'],
+    getPossibleOperationForField: () => ({ scale: 'ratio' }),
+  } as unknown as GenericOperationDefinition,
+  cumulative_sum: {
+    input: 'fullReference',
+    getPossibleOperationForField: () => ({ scale: 'ratio' }),
   } as unknown as GenericOperationDefinition,
-  cumulative_sum: { input: 'fullReference' } as GenericOperationDefinition,
 };
 
 describe('formula', () => {
@@ -1230,6 +1254,30 @@ invalid: "
       }
     });
 
+    it('returns errors if the returned type of an operation is not supported by Formula', () => {
+      // check only "valid" operations which are strictly not supported by Formula
+      // as for last_value with ordinal data
+      const formulas = [
+        { formula: 'last_value(dest)' },
+        { formula: 'terms(dest)' },
+        { formula: 'moving_average(last_value(dest), window=7)', errorFormula: 'last_value(dest)' },
+      ];
+      for (const { formula, errorFormula } of formulas) {
+        expect(
+          formulaOperation.getErrorMessage!(
+            getNewLayerWithFormula(formula),
+            'col1',
+            indexPattern,
+            operationDefinitionMap
+          )
+        ).toEqual([
+          `The return value type of the operation ${
+            errorFormula ?? formula
+          } is not supported in Formula.`,
+        ]);
+      }
+    });
+
     // there are 4 types of errors for math functions:
     // * no argument passed
     // * too many arguments passed

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts

@@ -76,6 +76,10 @@ interface ValidationErrors {
     message: string;
     type: { operation: string; text: string; type: string };
   };
+  wrongReturnedType: {
+    message: string;
+    type: { text: string };
+  };
 }
 
 type ErrorTypes = keyof ValidationErrors;
@@ -308,6 +312,13 @@ function getMessageFromId<K extends ErrorTypes>({
         values: { operation: out.operation, text: out.text, type: out.type },
       });
       break;
+    case 'wrongReturnedType':
+      message = i18n.translate('xpack.lens.indexPattern.formulaOperationWrongReturnedType', {
+        defaultMessage:
+          'The return value type of the operation {text} is not supported in Formula.',
+        values: { text: out.text },
+      });
+      break;
     // case 'mathRequiresFunction':
     //   message = i18n.translate('xpack.lens.indexPattern.formulaMathRequiresFunctionLabel', {
     //     defaultMessage; 'The function {name} requires an Elasticsearch function',
@@ -602,6 +613,7 @@ function runFullASTValidation(
             const fieldErrors = validateFieldArguments(node, variables, {
               isFieldOperation: true,
               firstArg,
+              returnedType: getReturnedType(nodeOperation, indexPattern, firstArg),
             });
             if (fieldErrors.length) {
               errors.push(...fieldErrors);
@@ -711,6 +723,7 @@ function runFullASTValidation(
           const fieldErrors = validateFieldArguments(node, variables, {
             isFieldOperation: false,
             firstArg,
+            returnedType: undefined,
           });
           if (fieldErrors.length) {
             errors.push(...fieldErrors);
@@ -775,6 +788,25 @@ export function getWrongTypeParams(
   );
 }
 
+function getReturnedType(
+  operation: OperationDefinition<IndexPatternColumn, 'field'>,
+  indexPattern: IndexPattern,
+  firstArg: TinymathAST
+) {
+  const variables = findVariables(firstArg);
+  if (variables.length !== 1) {
+    return;
+  }
+  const field = indexPattern.getFieldByName(getValueOrName(variables[0]) as string);
+  // while usually this is used where it is safe, as generic function it should check anyway
+  if (!field) {
+    return;
+  }
+  // here we're validating the support of the returned type for Formula, not for the operation itself
+  // that is already handled indipendently by the operation. So return the scale type
+  return operation.getPossibleOperationForField(field)?.scale;
+}
+
 function getDuplicateParams(params: TinymathNamedArgument[] = []) {
   const uniqueArgs = Object.create(null);
   for (const { name } of params) {
@@ -898,7 +930,15 @@ export function validateMathNodes(root: TinymathAST, missingVariableSet: Set<str
 function validateFieldArguments(
   node: TinymathFunction,
   variables: Array<string | number | TinymathVariable>,
-  { isFieldOperation, firstArg }: { isFieldOperation: boolean; firstArg: TinymathAST }
+  {
+    isFieldOperation,
+    firstArg,
+    returnedType,
+  }: {
+    isFieldOperation: boolean;
+    firstArg: TinymathAST;
+    returnedType: 'ratio' | 'ordinal' | 'interval' | undefined;
+  }
 ) {
   const fields = variables.filter(
     (arg) => isArgumentValidType(arg, 'variable') && !isMathNode(arg)
@@ -920,6 +960,19 @@ function validateFieldArguments(
       })
     );
   }
+  if (isFieldOperation && fields.length === 1 && fields[0] === firstArg) {
+    if (returnedType === 'ordinal') {
+      errors.push(
+        getMessageFromId({
+          messageId: 'wrongReturnedType',
+          values: {
+            text: node.text ?? `${node.name}(${getValueOrName(firstArg)})`,
+          },
+          locations: node.location ? [node.location] : [],
+        })
+      );
+    }
+  }
   if (!isFieldOperation && fields.length) {
     errors.push(
       getMessageFromId({

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx

@@ -492,7 +492,7 @@ describe('last_value', () => {
         'Field notExisting was not found',
       ]);
     });
-    it('shows error message  if the sortField does not exist in index pattern', () => {
+    it('shows error message if the sortField does not exist in index pattern', () => {
       errorLayer = {
         ...errorLayer,
         columns: {
@@ -509,6 +509,20 @@ describe('last_value', () => {
         'Field notExisting was not found',
       ]);
     });
+    it('shows error message if the sourceField is of unsupported type', () => {
+      errorLayer = {
+        ...errorLayer,
+        columns: {
+          col1: {
+            ...errorLayer.columns.col1,
+            sourceField: 'timestamp',
+          } as LastValueIndexPatternColumn,
+        },
+      };
+      expect(lastValueOperation.getErrorMessage!(errorLayer, 'col1', indexPattern)).toEqual([
+        'Field timestamp is of the wrong type',
+      ]);
+    });
     it('shows error message if the sortField is not date', () => {
       errorLayer = {
         ...errorLayer,

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx

@@ -206,7 +206,8 @@ export const lastValueOperation: OperationDefinition<LastValueIndexPatternColumn
       newField &&
         newField.type === column.dataType &&
         !newField.aggregationRestrictions &&
-        newTimeField?.type === 'date'
+        newTimeField?.type === 'date' &&
+        supportedTypes.has(newField.type)
     );
   },