Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More cleanup of array functions #2058

Merged
merged 26 commits into from
Dec 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
7fd118d
make wrapDef a util
mtoy-googly-moogly Dec 17, 2024
1d58a8f
move repeat out of standard library
mtoy-googly-moogly Dec 17, 2024
e91435c
cleanup wrapDef a little more
mtoy-googly-moogly Dec 17, 2024
727fd86
even more wrapDef cleanup
mtoy-googly-moogly Dec 17, 2024
da50ddf
fix oopsie
mtoy-googly-moogly Dec 17, 2024
3e8286c
maybe cleaner still?
mtoy-googly-moogly Dec 17, 2024
d1e9479
fix find/replace mistake
mtoy-googly-moogly Dec 17, 2024
ab5a09a
rename arg
mtoy-googly-moogly Dec 17, 2024
8fbb030
make all param names into strings
mtoy-googly-moogly Dec 17, 2024
851c63a
fix trino reverse
mtoy-googly-moogly Dec 17, 2024
692396d
moved todos to docs issue
mtoy-googly-moogly Dec 17, 2024
1688802
Merge branch 'main' into array-func-cleanup
mtoy-googly-moogly Dec 17, 2024
e7bc100
remove extra test for % operator
mtoy-googly-moogly Dec 17, 2024
14103b4
put the cast back into trino reverse
mtoy-googly-moogly Dec 17, 2024
07119c6
go ahead and test reverse for arrays on presto
mtoy-googly-moogly Dec 17, 2024
83a3822
try again to get reverse with generic correct
mtoy-googly-moogly Dec 18, 2024
750b1ac
shared string_reverse
mtoy-googly-moogly Dec 18, 2024
0a23532
maybe finally fix reverse?
mtoy-googly-moogly Dec 18, 2024
a11e45b
use defwrap to cleanup some errors
mtoy-googly-moogly Dec 18, 2024
1621aaf
another minor code tweak to wrapDef
mtoy-googly-moogly Dec 18, 2024
7514bb6
-cleverness, +readability
mtoy-googly-moogly Dec 18, 2024
a76e4aa
add comment
mtoy-googly-moogly Dec 18, 2024
001f24f
comment edit
mtoy-googly-moogly Dec 18, 2024
5f23388
comment explaining dialect split
mtoy-googly-moogly Dec 18, 2024
26a921d
fix T comment
mtoy-googly-moogly Dec 18, 2024
01b7333
Extend wrapDef to def and overdef (#2060)
mtoy-googly-moogly Dec 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions packages/malloy/src/dialect/duckdb/dialect_functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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']},
Expand Down Expand Up @@ -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'),
};
35 changes: 32 additions & 3 deletions packages/malloy/src/dialect/functions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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: {},
Expand All @@ -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 = {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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'},
Expand Down Expand Up @@ -718,7 +711,6 @@ export const MALLOY_STANDARD_FUNCTIONS: MalloyStandardFunctionDefinitions = {
rand,
regexp_extract,
replace,
reverse,
round,
rtrim,
sign,
Expand Down
85 changes: 85 additions & 0 deletions packages/malloy/src/dialect/functions/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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};
}
11 changes: 3 additions & 8 deletions packages/malloy/src/dialect/mysql/dialect_functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'}},
Expand Down Expand Up @@ -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'),
};
11 changes: 3 additions & 8 deletions packages/malloy/src/dialect/postgres/dialect_functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'}},
Expand Down Expand Up @@ -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'),
};
11 changes: 3 additions & 8 deletions packages/malloy/src/dialect/snowflake/dialect_functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(',
Expand Down Expand Up @@ -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'),
};
10 changes: 3 additions & 7 deletions packages/malloy/src/dialect/standardsql/dialect_functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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'),
};
Loading
Loading