More cleanup of array functions (#2058)

* make wrapDef a util

* move repeat out of standard library

* cleanup wrapDef a little more

* even more wrapDef cleanup

* fix oopsie

* maybe cleaner still?

* fix find/replace mistake

* rename arg

* make all param names into strings

* fix trino reverse

* moved todos to docs issue

* remove extra test for % operator

* put the cast back into trino reverse

* go ahead and test reverse for arrays on presto

* try again to get reverse with generic correct

* shared string_reverse

* maybe finally fix reverse?

* use defwrap to cleanup some errors

* another minor code tweak to wrapDef

* -cleverness, +readability

* add comment

* comment edit

* comment explaining dialect split

* fix T comment

* Extend wrapDef to def and overdef (#2060)

* experiment with overdef

* fix overdef()

* mark bitwise_aggs as symmetric

* move all the hll functiosn to ... def

* make preto-only all def()

* delete overdef

* stop using def for hll functions

* fix presto funcs

* cleanup sequence()

Author: mtoy-googly-moogly

packages/malloy/src/dialect/duckdb/dialect_functions.ts

@@ -9,14 +9,9 @@ import {
   DefinitionBlueprint,
   DefinitionBlueprintMap,
   OverloadedDefinitionBlueprint,
+  def,
 } from '../functions/util';
 
-const repeat: DefinitionBlueprint = {
-  takes: {'str': 'string', 'n': 'number'},
-  returns: 'string',
-  impl: {function: 'REPEAT'},
-};
-
 const list_extract: DefinitionBlueprint = {
   takes: {'value': {array: {generic: 'T'}}, 'index': 'number'},
   generic: {'T': ['any']},
@@ -103,9 +98,10 @@ export const DUCKDB_DIALECT_FUNCTIONS: DefinitionBlueprintMap = {
   count_approx,
   dayname,
   to_timestamp,
-  repeat,
   string_agg,
   string_agg_distinct,
   to_seconds,
   date_part,
+  ...def('repeat', {'str': 'string', 'n': 'number'}, 'string'),
+  ...def('reverse', {'str': 'string'}, 'string'),
 };

packages/malloy/src/dialect/functions/README.md

@@ -31,14 +31,14 @@ const abs: DefinitionFor<Standard['my_function']> = {
     * `${...param}` is expanded to a spread fragment, which is expanded to a comma-separated list of arguments for a variadic parameter
     * `${order_by:}` and `${limit:}` expand to `aggregate_order_by` and `aggregate_limit` fragments respectively
   * `{ expr: { node: "node_type", ... } }`, for cases where the `function` or `sql` options are insufficiently flexible; generates SQL for an arbitrary node, which may include parameter fragments, spread fragments (optionally with prefix and suffix), and order by or limit fragments.
-* `generic` specifies that the overload is generic over some types; it should be an array `['T', types]`, where `'T'` is the name of the generic, and `types` is an array of Malloy types: e.g. `generic: ['T', ['string', 'number', 'timestamp', 'date', 'json']],`. Note that currently we only allow one generic per overload, since that's all we need right now. Under the hood, this creates multiple overloads, one for each type in the generic. Eventually we may make the function system smarter and understand generics more deeply.
+* `generic` specifies that the overload is generic over some types; it should be an object `{'T': types}`, where `'T'` is the name of the generic, and `types` is an array of Malloy types: e.g. `generic: {'T', ['string', 'number', 'timestamp', 'date', 'json']},`. Note that currently we only allow one generic per overload, since that's all we need right now. Under the hood, this creates multiple overloads, one for each type in the generic. Eventually we may make the function system smarter and understand generics more deeply.
 * `supportsOrderBy` is for aggregate functions (e.g. `string_agg`) which can accept an `{ order_by: value }`. `false` is the default value. `true` indicates that any column may be used in the `order_by`. A value of `'default_only'` means that only the more limited `{ order_by: asc }` syntax is allowed.
 * `supportsLimit`: is for aggregate functions (e.g. `string_agg`) which can accept a `{ limit: 10 }`. Default `false`.
 * `isSymmetric` is for aggregate functions to indicate whether they are symmetric. Default `false`.
 
 A function with multiple overloads is defined like:
 
-```
+```TypeScript
 const concat: DefinitionFor<Standard['concat']> = {
   'empty': {
     takes: {},
@@ -55,7 +55,7 @@ const concat: DefinitionFor<Standard['concat']> = {
 
 Each dialect may override the standard implementations (that is, the `impl` part only). To do so:
 
-```
+```TypeScript
 import {MalloyStandardFunctionImplementations as OverrideMap} from '../functions/malloy_standard_functions';
 
 export const DIALECT_OVERRIDES: OverrideMap = {
@@ -106,6 +106,35 @@ export const DIALECT_FUNCTIONS: DefinitionBlueprintMap = {
 };
 ```
 
+### The `def()` experiment
+There is also an experimental shortcut for simple wrapper definitions where the name of the function in SQL and in Malloy is the same, and the definition is not overloaded. Here's an example if using `def` to define the string length function ...
+
+Instead of writing
+
+```ts
+const length: DefinitionBluePrint = {
+  takes: {str: 'string'},
+  returns: 'number',
+  impl: {function: 'LENGTH'},
+}
+export const DIALECT_FUNCTIONS: DefinitionBlueprintMap = {
+  my_function,
+  length
+}
+```
+
+The shortcut looks like this ...
+
+```ts
+export const DIALECT_FUNCTIONS: DefinitionBlueprintMap = {
+  my_function,
+  ...def('length', {str: 'string'}, 'number')
+};
+```
+
+We are waiting on user feedback on this experiment. While these are simpler to write, they are not simpler to read for a human scanning the file for the definition of a function.
+
+### Dialect Implementation
 The `Dialect` implementation then implements `getDialectFunctions`. You should use `expandBlueprintMap` to expand the function blueprints into `DialectFunctionOverloadDef`s.
 
 ```ts

packages/malloy/src/dialect/functions/malloy_standard_functions.ts

@@ -60,7 +60,6 @@ type Standard = {
   regexp_extract: D;
   string_repeat: D;
   replace: {string: D; regular_expression: D};
-  reverse: D;
   round: {to_integer: D; to_precision: D};
   rtrim: {whitespace: D; characters: D};
   sign: D;
@@ -339,12 +338,6 @@ const replace: DefinitionFor<Standard['replace']> = {
   },
 };
 
-const reverse: DefinitionFor<Standard['reverse']> = {
-  takes: {'value': 'string'},
-  returns: 'string',
-  impl: {function: 'REVERSE'},
-};
-
 const round: DefinitionFor<Standard['round']> = {
   'to_integer': {
     takes: {'value': 'number'},
@@ -718,7 +711,6 @@ export const MALLOY_STANDARD_FUNCTIONS: MalloyStandardFunctionDefinitions = {
   rand,
   regexp_extract,
   replace,
-  reverse,
   round,
   rtrim,
   sign,

packages/malloy/src/dialect/functions/util.ts

@@ -723,3 +723,88 @@ export function expandOverrideMapFromBase(
   }
   return map;
 }
+
+/**
+ * Walks a type and returns all the generic references
+ * @param tdbp A type
+ */
+function* findGenerics(
+  tdbp: TypeDescBlueprint
+): IterableIterator<NamedGeneric> {
+  if (typeof tdbp !== 'string') {
+    if ('generic' in tdbp) {
+      yield tdbp;
+    } else if ('record' in tdbp) {
+      for (const recType of Object.values(tdbp.record)) {
+        yield* findGenerics(recType);
+      }
+    } else {
+      for (const leaflet of [
+        'array',
+        'literal',
+        'measure',
+        'dimension',
+        'measure',
+        'constant',
+        'cacluation',
+      ]) {
+        if (leaflet in tdbp) {
+          yield* findGenerics(tdbp[leaflet]);
+          return;
+        }
+      }
+    }
+  }
+}
+
+/**
+ * Shortcut for non overloaded functions definitions. Default implementation
+ * will be the function name turned to upper case. Default type for
+ * any generics encountered will be `['any']`. Both of these can be over-ridden
+ * in the `options` parameter.
+ *
+ * The two implict defaults (which can be over-ridden) are that the
+ * impl: will be the upper case version of the function name, and that
+ * any generic reference will be of type 'any'.
+ *
+ * USAGE:
+ *
+ *     ...def('func_name', {'arg0': 'type0', 'arg1': 'type1'}, 'return-type')
+ *
+ * @param name name of function
+ * @param takes Record<Argument blueprint>
+ * @param returns Return Blueprint
+ * @param options Everything from a `DefinitionBlueprint` except `takes` and `returns`
+ * @returns dot dot dot able blueprint definition
+ */
+export function def(
+  name: string,
+  takes: Record<string, TypeDescBlueprint>,
+  returns: TypeDescBlueprint,
+  options: Partial<Omit<DefinitionBlueprint, 'takes' | 'returns'>> = {}
+) {
+  let anyGenerics = false;
+  const generic: {[name: string]: TypeDescElementBlueprintOrNamedGeneric[]} =
+    {};
+  for (const argType of Object.values(takes)) {
+    for (const genericRef of findGenerics(argType)) {
+      generic[genericRef.generic] = ['any'];
+      anyGenerics = true;
+    }
+  }
+  // We have found all the generic references and given them all
+  // T: ['any']. Use this as a default if the options section
+  // doesn't provide types for the generics.
+  if (anyGenerics) {
+    if (options.generic === undefined) {
+      options.generic = generic;
+    }
+  }
+  const newDef: DefinitionBlueprint = {
+    takes,
+    returns,
+    impl: {function: name.toUpperCase()},
+    ...options,
+  };
+  return {[name]: newDef};
+}

packages/malloy/src/dialect/mysql/dialect_functions.ts

@@ -6,17 +6,11 @@
  */
 
 import {
-  DefinitionBlueprint,
   DefinitionBlueprintMap,
   OverloadedDefinitionBlueprint,
+  def,
 } from '../functions/util';
 
-const repeat: DefinitionBlueprint = {
-  takes: {'str': 'string', 'n': 'number'},
-  returns: 'string',
-  impl: {function: 'REPEAT'},
-};
-
 const string_agg: OverloadedDefinitionBlueprint = {
   default_separator: {
     takes: {'value': {dimension: 'string'}},
@@ -59,5 +53,6 @@ const string_agg_distinct: OverloadedDefinitionBlueprint = {
 export const MYSQL_DIALECT_FUNCTIONS: DefinitionBlueprintMap = {
   string_agg,
   string_agg_distinct,
-  repeat,
+  ...def('repeat', {'str': 'string', 'n': 'number'}, 'string'),
+  ...def('reverse', {'str': 'string'}, 'string'),
 };

packages/malloy/src/dialect/postgres/dialect_functions.ts

@@ -6,17 +6,11 @@
  */
 
 import {
-  DefinitionBlueprint,
   DefinitionBlueprintMap,
   OverloadedDefinitionBlueprint,
+  def,
 } from '../functions/util';
 
-const repeat: DefinitionBlueprint = {
-  takes: {'str': 'string', 'n': 'number'},
-  returns: 'string',
-  impl: {function: 'REPEAT'},
-};
-
 const string_agg: OverloadedDefinitionBlueprint = {
   default_separator: {
     takes: {'value': {dimension: 'string'}},
@@ -59,5 +53,6 @@ const string_agg_distinct: OverloadedDefinitionBlueprint = {
 export const POSTGRES_DIALECT_FUNCTIONS: DefinitionBlueprintMap = {
   string_agg,
   string_agg_distinct,
-  repeat,
+  ...def('repeat', {'str': 'string', 'n': 'number'}, 'string'),
+  ...def('reverse', {'str': 'string'}, 'string'),
 };

packages/malloy/src/dialect/snowflake/dialect_functions.ts

@@ -7,19 +7,13 @@
 
 import {AggregateOrderByNode} from '../../model';
 import {
-  DefinitionBlueprint,
+  def,
   DefinitionBlueprintMap,
   OverloadedDefinitionBlueprint,
   arg as a,
   sql,
 } from '../functions/util';
 
-const repeat: DefinitionBlueprint = {
-  takes: {'str': 'string', 'n': 'number'},
-  returns: 'string',
-  impl: {function: 'REPEAT'},
-};
-
 const order_by: AggregateOrderByNode = {
   node: 'aggregate_order_by',
   prefix: ' WITHIN GROUP(',
@@ -68,5 +62,6 @@ const string_agg_distinct: OverloadedDefinitionBlueprint = {
 export const SNOWFLAKE_DIALECT_FUNCTIONS: DefinitionBlueprintMap = {
   string_agg,
   string_agg_distinct,
-  repeat,
+  ...def('repeat', {'str': 'string', 'n': 'number'}, 'string'),
+  ...def('reverse', {'str': 'string'}, 'string'),
 };

packages/malloy/src/dialect/standardsql/dialect_functions.ts

@@ -6,17 +6,12 @@
  */
 
 import {
+  def,
   DefinitionBlueprint,
   DefinitionBlueprintMap,
   OverloadedDefinitionBlueprint,
 } from '../functions/util';
 
-const repeat: DefinitionBlueprint = {
-  takes: {'str': 'string', 'n': 'number'},
-  returns: 'string',
-  impl: {function: 'REPEAT'},
-};
-
 const date_from_unix_date: DefinitionBlueprint = {
   takes: {'unix_date': 'number'},
   returns: 'date',
@@ -74,5 +69,6 @@ export const STANDARDSQL_DIALECT_FUNCTIONS: DefinitionBlueprintMap = {
   date_from_unix_date,
   string_agg,
   string_agg_distinct,
-  repeat,
+  ...def('repeat', {'str': 'string', 'n': 'number'}, 'string'),
+  ...def('reverse', {'str': 'string'}, 'string'),
 };