Add variable field support in aggregations by Atlas07 · Pull Request #18 · ahrefs/esgg

Atlas07 · 2026-03-21T14:55:08Z

Summary

Addresses #3

Adds $(var:field:<type>) syntax for parameterizing the "field" value in aggregations (e.g., $(group_by:field:string))
The variable becomes a string input parameter (the ES field name passed at runtime)
The type annotation (string, int, float, int64) determines the output bucket key type
Allows a single query template to be reused across multiple ES fields instead of duplicating files

Changes

tjson.ml: Extended variable parser to recognize $(var:field:<type>) alongside existing $(var:list)
common.ml: Added Field_var constructor to value type, parse_field_type helper, and typeof_value handling
aggregations.ml: Wired Field_var through aggregation analysis and constraint resolution
query.ml: Handle Field_num (Field_var _) in constraint validation

Test Plan

New regression test field_var_agg with terms aggregation using $(group_by:field:string)
All 23 tests pass (22 existing + 1 new)
Smoke tested with exact syntax from the issue

Support $(var:field:<type>) syntax alongside existing $(var:list). No behavioral changes yet — field_type is parsed but unused. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Field_var carries a Tjson.var and a simple_type parsed from the field type annotation. typeof_value returns Simple typ directly, bypassing the mapping lookup. Also adds parse_field_type helper to convert user-facing type strings ("string", "int", "float", "int64") to simple_type. Note: the value type definition was moved after pp_simple_type because [@@deriving show] on Field_var needs the printer in scope. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Parse $(var:field:<type>) in aggregation field position - Register field variables as string input parameters - Skip numeric/date field validation for dynamic fields Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Tests $(group_by:field:string) syntax: verifies string input parameter and string bucket key output type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add field variable section under Variables with usage example and supported type annotations.

Use descriptive names like _typ, _name, _metric, _keys etc. instead of bare _ in pattern matches added by this PR.

rr0gi · 2026-03-23T18:08:02Z

+let parse_field_type = function
+  | "string" -> String
+  | "int" -> Int
+  | "float" -> Double
+  | "int64" -> Int64
+  | s -> fail "unsupported field type annotation %S, expected: string, int, float, int64" s


why not simple_of_es_type above

Resolved in 8e044e4

rr0gi · 2026-03-23T18:15:45Z

+      match String.nsplit s ":" with
+      | [name] -> name, false, None
+      | [name; "list"] -> name, true, None
+      | [name; "field"; typ] -> name, false, Some typ


i think better syntax is just name:type? maybe it will be useful in more contexts than dynamic field and overall there is no semantic importance to it being field, it is currently a field type only by the virtue of place where it is used

Resolved in 45a09a6

rr0gi · 2026-03-23T18:18:21Z

    | Bool | Json as t -> eprintfn "W: field %S expected to be numeric, but has type %s" f (show_simple_type t)
    end
  | Field_num (Script _) -> ()
+  | Field_num (Field_var (_v, _typ)) -> () (* cannot validate dynamic field type *)


but we know the type and can validate it same as Field f above?

Resolved in 5ae15fc

rr0gi · 2026-03-23T18:24:40Z

+  let field_var_cstrs = match agg with
+    | Dynamic _dv -> []
+    | Static agg ->
+      let vs = match agg with
+        | Simple_metric (_metric, v) -> [v]
+        | Value_count v | Cardinality v
+        | Histogram v | Range v -> [v]
+        | Range_keyed (v, _keys) -> [v]
+        | Terms { term; _ } | Significant_terms { term; _ } | Significant_text { term; _ } -> [term]
+        | Date_histogram { on; _ } | Date_range { on; _ } -> [on]
+        | Multi_terms { terms; _ } -> terms
+        | Weighted_avg { value; weight } -> [value.value; weight.value]
+        | Filter _q -> []
+        | Filters _fs -> []
+        | Filters_dynamic _dv -> []
+        | Top_hits _th -> []
+        | Nested _path -> []
+        | Reverse_nested _rpath -> []
+        | Bucket_sort _bs -> []
+        | Cumulative_sum _cp -> []
+      in
+      List.concat_map field_var_constraints vs
+  in


this can be done in the above match where all the constraints are computed already

Resolved in 05db4fd

Type annotations now use ES type names (keyword, long, double) instead of custom names (string, int, float). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The :field: segment was redundant — the type annotation is useful regardless of context, and the usage site already determines semantics. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The annotated type is known at codegen time, so we can warn when a non-numeric type is used in a numeric context — same as Field. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Eliminates redundant second match on aggregation type. Each arm now handles its own field_var_constraints inline, matching the existing pattern for on_int_var and Field_num/Field_date constraints. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

rr0gi

please make fixes and merge

rr0gi · 2026-03-26T13:12:25Z

 let dummy_expunge = Dict ["dummy_expunge", Simple Json]

+let field_var_constraints = function
+  | Field_var (v, _typ) -> [On_var (v, Eq_type String)]


i think this warrants a comment that String here is for the variable value itself, not for the aggregation result, to avoid confusion

Resolved in 465917e

rr0gi · 2026-03-26T13:15:16Z

+
+The type annotation determines the output bucket key type. The variable itself becomes a `string` input parameter — the caller passes the ES field name at runtime (e.g., `"keyword_en"`).
+
+Supported type annotations: any ES type accepted by `simple_of_es_type` (`keyword`, `text`, `long`, `double`, `float`, `boolean`, `date`, `int64`, `ip`, `murmur3`).


this is external documentation, they don't know what is simple_of_es_type.

Suggested change

Supported type annotations: any ES type accepted by `simple_of_es_type` (`keyword`, `text`, `long`, `double`, `float`, `boolean`, `date`, `int64`, `ip`, `murmur3`).

Type can be any ES type - `keyword`, `text`, `long`, `double`, `float`, `boolean`, `date`, `int64`, `ip`, `murmur3`.

Resolved in 6c14bde

rr0gi · 2026-03-26T13:17:48Z

 let debug_dump = false

-type var = { optional : bool; list : bool; name : string } [@@deriving show]
+type var = { optional : bool; list : bool; name : string; field_type : string option } [@@deriving show]


Suggested change

type var = { optional : bool; list : bool; name : string; field_type : string option } [@@deriving show]

type var = { optional : bool; list : bool; name : string; type_ : string option } [@@deriving show]

but at this point will be cleaner to get rid of list bool and rename type_ to hint and introduce Common.var parsed from generic Tjson.var, not in this PR

Resolved in e20ff5b

rr0gi · 2026-03-26T13:18:53Z

+  let opt_suffix = if optional then "?" else "" in
+  let name_s = name ^ opt_suffix in
+  match field_type with
+  | Some ft -> sprintf "$(%s:%s)" name_s ft


natural qn is what if it is both list and has type, guess can assert for now to be clear

Resolved in af75b0f

String is the type of the variable value (field name passed at runtime), not the aggregation result type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

External users don't know what simple_of_es_type is. List supported types directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The field is no longer field-specific after the syntax simplification. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The parser branches already prevent this, but the explicit check makes the invariant visible. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Tests $(bucket_field:long) in a histogram aggregation, exercising the Field_num constraint validation path and float bucket key output. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: Vitalii Shapoval <vitalii_shapoval@ukr.net> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Atlas07 and others added 6 commits March 21, 2026 14:17

Add field_type to Tjson.var record and extend variable parser

181a211

Support $(var:field:<type>) syntax alongside existing $(var:list). No behavioral changes yet — field_type is parsed but unused. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add regression test for field variable in terms aggregation

fd4e59c

Tests $(group_by:field:string) syntax: verifies string input parameter and string bucket key output type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Document $(var:field:<type>) syntax in README

66e50ab

Add field variable section under Variables with usage example and supported type annotations.

Replace bare _ wildcards with named wildcards for readability

d713514

Use descriptive names like _typ, _name, _metric, _keys etc. instead of bare _ in pattern matches added by this PR.

rr0gi requested changes Mar 23, 2026

View reviewed changes

Atlas07 and others added 4 commits March 24, 2026 01:11

Reuse simple_of_es_type instead of separate parse_field_type

8e044e4

Type annotations now use ES type names (keyword, long, double) instead of custom names (string, int, float). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Simplify variable syntax from $(var:field:<type>) to $(var:<type>)

45a09a6

The :field: segment was redundant — the type annotation is useful regardless of context, and the usage site already determines semantics. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Validate Field_var type in Field_num constraint

5ae15fc

The annotated type is known at codegen time, so we can warn when a non-numeric type is used in a numeric context — same as Field. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Atlas07 requested a review from rr0gi March 26, 2026 12:35

rr0gi approved these changes Mar 26, 2026

View reviewed changes

Atlas07 and others added 5 commits March 26, 2026 14:51

Add clarifying comment to field_var_constraints Eq_type String

465917e

String is the type of the variable value (field name passed at runtime), not the aggregation result type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Remove internal function reference from README

6c14bde

External users don't know what simple_of_es_type is. List supported types directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Rename field_type to type_ in Tjson.var

e20ff5b

The field is no longer field-specific after the syntax simplification. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Assert list and type_ are mutually exclusive in make_var

af75b0f

The parser branches already prevent this, but the explicit check makes the invariant visible. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add regression test for numeric-type field variable in histogram

93f14c3

Tests $(bucket_field:long) in a histogram aggregation, exercising the Field_num constraint validation path and float bucket key output. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Atlas07 merged commit 357d5ac into ahrefs:master Mar 26, 2026

Atlas07 mentioned this pull request Mar 26, 2026

Add Support for using variables as field values in Terms Aggregation #3

Closed

rr0gi added a commit that referenced this pull request Mar 27, 2026

Add variable field support in aggregations (#18)

a0396e9

Author: Vitalii Shapoval <vitalii_shapoval@ukr.net> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

rr0gi pushed a commit that referenced this pull request Mar 27, 2026

Add variable field support in aggregations (#18)

27cdc66

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>


		The type annotation determines the output bucket key type. The variable itself becomes a `string` input parameter — the caller passes the ES field name at runtime (e.g., `"keyword_en"`).

		Supported type annotations: any ES type accepted by `simple_of_es_type` (`keyword`, `text`, `long`, `double`, `float`, `boolean`, `date`, `int64`, `ip`, `murmur3`).

	Supported type annotations: any ES type accepted by `simple_of_es_type` (`keyword`, `text`, `long`, `double`, `float`, `boolean`, `date`, `int64`, `ip`, `murmur3`).
	Type can be any ES type - `keyword`, `text`, `long`, `double`, `float`, `boolean`, `date`, `int64`, `ip`, `murmur3`.

	type var = { optional : bool; list : bool; name : string; field_type : string option } [@@deriving show]
	type var = { optional : bool; list : bool; name : string; type_ : string option } [@@deriving show]

Conversation

Atlas07 commented Mar 21, 2026

Summary

Changes

Test Plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rr0gi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants