[daniellansun]

JIRA

GROOVY-12724 — GINQ: support static compilation and a fluent query API

Summary

GQ, GQL, and @GQ can be used from a @CompileStatic or @TypeChecked scope. The SQL-like syntax is still lowered to the collection runtime. Under static checking that lowered tree is not run when the expression is evaluated: a LazyQueryable builds it on the first read (toList, count, and the other terminals). Locals are copied at the expression site (same binding as dynamic GQ); sources stay closed until a terminal.

Ginq.from adds a deferred, generic fluent chain on the same in-memory runtime. Fluent closure overloads carry the element type into where / select / joins / aggregates via @ClosureParams where inference is sound; remaining sites keep explicit parameter types.

Stacked on GEP-26 SQL window work (#3167 / gep26-sql-window).

Known limits

• Stored untyped Closure variables still need a SAM target type for parameter inference (same as Java/C#).
• Raw Query parameters/fields cannot recover a safe element type.
• Names bound by from inside GQ / GQL / @GQ stay dynamic by design; callers that need checked element types use Ginq.from.

Test

• :groovy-ginq:test and :groovy-ginq-sql:test green locally
• RAT license header fixed on the aggregate SPI file
• Checkstyle / CodeNarc gates green after import cleanup

[codecov-commenter]

Codecov Report

❌ Patch coverage is 79.07732% with 322 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.5259%. Comparing base (856b28a) to head (c56d922).
⚠️ Report is 20 commits behind head on master.

Files with missing lines	Patch %	Lines
...n/groovy/org/apache/groovy/ginq/GinqRowChecks.java	62.4729%	74 Missing and 99 partials ⚠️
...roovy/org/apache/groovy/ginq/GinqStaticBridge.java	83.0508%	29 Missing and 41 partials ⚠️
...a/org/codehaus/groovy/ast/tools/GenericsUtils.java	60.3448%	8 Missing and 15 partials ⚠️
.../codehaus/groovy/ast/tools/WideningCategories.java	40.0000%	5 Missing and 10 partials ⚠️
...roovy/org/apache/groovy/ginq/fluent/GinqQuery.java	93.5065%	4 Missing and 6 partials ⚠️
...ehaus/groovy/classgen/asm/TypeSignatureWriter.java	63.6364%	2 Missing and 6 partials ⚠️
...ovider/collection/runtime/QueryableCollection.java	91.6667%	3 Missing and 4 partials ⚠️
...e/groovy/ginq/transform/GinqASTTransformation.java	81.8182%	2 Missing and 4 partials ⚠️
...sc/StaticTypesMethodReferenceExpressionWriter.java	0.0000%	2 Missing and 1 partial ⚠️
...oovy/ginq/provider/collection/GinqAstWalker.groovy	88.8889%	0 Missing and 3 partials ⚠️
... and 3 more

Additional details and impacted files

@@                Coverage Diff                 @@
##               master      #3259        +/-   ##
==================================================
+ Coverage     74.4429%   74.5259%   +0.0831%     
- Complexity      44774      45446       +672     
==================================================
  Files            1702       1710         +8     
  Lines          151891     153509      +1618     
  Branches        29091      29479       +388     
==================================================
+ Hits           113072     114404      +1332     
- Misses          28413      28522       +109     
- Partials        10406      10583       +177     

Files with missing lines	Coverage Δ
...va/org/codehaus/groovy/control/ErrorCollector.java	67.0588% <100.0000%> (ø)
...roovy/transform/stc/StaticTypeCheckingSupport.java	79.6377% <100.0000%> (+0.3623%)	⬆️
...roovy/transform/stc/StaticTypeCheckingVisitor.java	88.0335% <100.0000%> (+0.0631%)	⬆️
.../groovy-ginq/src/main/groovy/groovy/ginq/Ginq.java	100.0000% <100.0000%> (ø)
...groovy-ginq/src/main/groovy/groovy/ginq/Query.java	100.0000% <100.0000%> (ø)
...vy/org/apache/groovy/ginq/GinqGroovyMethods.groovy	81.2500% <100.0000%> (+0.6048%)	⬆️
...vy/org/apache/groovy/ginq/fluent/GinqGrouping.java	100.0000% <100.0000%> (ø)
...vy/ginq/provider/collection/runtime/Queryable.java	86.0465% <100.0000%> (+9.3798%)	⬆️
...provider/collection/runtime/QueryableHelper.groovy	71.6981% <100.0000%> (-1.7713%)	⬇️
...nq/provider/collection/runtime/SourceRecord.groovy	85.7143% <ø> (ø)
... and 13 more

... and 34 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[blackdrag]

Not a full review yet. I find it difficult to approach, because I miss something explaining the strategy for this implementation in more detail and concentrated. Of course I can conclude a lot from the code, but that does not make the intend clear in detail, only the implementation. In general I spotted a view things that look questionable at first glance. Like that adding of a phase operation, the local variables determination... in general most is tied to the lazy variant I would say. And it is not clear to me why you need lazy in this way either.
Also unclear is when you actually get static compilation. Or why some of the work done here cannot be left to the general infrastructure. Then there is the issue that this kind of feels like you try to extend the static compiler without extending the static compiler.

[daniellansun]

Thanks for the early notes. I added a concentrated strategy write-up at subprojects/groovy-ginq/STRATEGY.md (and aligned the user-guide CompileStatic section with it). Short answers to the points you raised:

Phase operation / local-variable walk. Both are gone on the current tip. The lowered query is no longer a synthetic ginqQuery$ method appended via CompilationUnit.addNewPhaseOperation. It stays a closure in the enclosing method. Capture of assigned locals is the normal closure-shared rule; read-only locals that must be snapshotted for a deferred call() are copied into an array when the expression runs (documented in STRATEGY.md). The hand-written local walk is removed.

Why lazy this way. Three separate deferrals, not one “lazy variant”:

1. Element pull was already deferred on Queryable (pipeline until a terminal).
2. Fluent Ginq.from records operators until a terminal (GinqQuery) — LINQ-to-Objects shape, not an expression tree.
3. Under @CompileStatic / @TypeChecked, static GQ wraps the lowered IIFE in LazyQueryable.defer so the pipeline’s call() runs on first read (closure still created at the expression to capture locals then). Dynamic GQ is unchanged. LazyQueryable is only that thunk.

When you get static compilation. Same as elsewhere: StaticCompileTransformation at instruction selection. @CompileStatic type-checks and statically compiles the enclosing method; the lowered query closure is marked DYNAMIC_RESOLUTION and emitted with the dynamic writer (StaticTypesClosureWriter already has that switch). @TypeChecked type-checks without static compile. No annotation / @CompileDynamic / SKIP leave the lowered tree fully dynamic. Row/property/aggregate bodies stay on Groovy’s object protocol on purpose (maps, beans, records).

General infrastructure vs a parallel mini-compiler. Method checking, bytecode, and written-local capture stay with the existing compiler. The only compiler delta is honouring DYNAMIC_RESOLUTION on a closure so the existing dynamic writer can compile it. Type rules for rows go through GinqRowChecks registered on the annotation’s extensions member — the normal STC extension point — not a subclass of StaticTypeCheckingVisitor / StaticCompilationVisitor, and not a new compile phase.

Happy to walk any remaining spot in the STRATEGY note or the tip.

[blackdrag]

@daniellansun This branch does not target main?

Disclaimer: These are conclusions based on the source, I did not actually execute any code. So maybe some points are incorrect.

(1)

@TypeChecked
class Example {
    @CompileDynamic
    def query(List<Map> rows) {
        GQ {
            from r in rows
            where r.value > 10
            select r.value
        }
    }
}

The method is still type-checked. Nevertheless, GinqStaticBridge.checked() returns false, so the bridge does not install its row-checking extension or wrap the lowered query closure.

The consequence is that the macro-generated code is presented to the checker without the adaptations required for dynamic GINQ row access.

@TypeChecked
class Example {
    @CompileStatic(TypeCheckingMode.SKIP)
    def query(List<Map> rows) {
        GQ {
            from r in rows
            select r.value
        }
    }
}

Is a similar case. @CompileDynamic and @CompileStatic(SKIP) must not be treated as equivalent to @TypeChecked(SKIP).

(2)

@CompileStatic
class Example {
    Object value

    def query(List<Map> rows) {
        GQ {
            from r in rows
            select r.value
        }
    }

    String unrelated() {
        value.mispelledProperty
    }
}

If I see right, then r.vlue would be accepted. GinqStaticBridge.attachRowChecks() attaches to the class level, which means it participates outside the GQ block. Most dynamic resolution is restricted to the marked GQ closure, but fallback handling for Object receivers is not restricted to that closure.
The post-visit subscript rewrite also operates outside it, imagine

    Object getValue(Object value) {
        value[0]
    }

instead of the method unrelated.

(3)
Static GQ needs to preserve the value visible when the expression is evaluated, even though construction of the underlying query pipeline is deferred.

int threshold = 10

GQ {
    from row in rows
    select {
        int threshold = 5
        row.value > threshold
    }
}

A name-based analysis has to distinguish the local threshold from the enclosing one. But the current global declared-name collection cannot represent that distinction. Similar for more complicated cases involving closure parameters, nested closures and declarations.
Also I am wondering if class values would be correctly recognized. I am thinking here of import aliases for example, that may make what seems to be a variable actually a class.
And STRATEGY.md says that the hand-written local-variable walk was removed, but the bridge still contains a scope collector and separate captured-variable analysis.

(4)

@GQ
def query(List rows) {
    from r in rows
    select r
}

If I understand right this undergoes return type widening and changes to Queryable. Also incompatibleSuperReturn does not ensure that the candidate actually represents the same method signature, nor does it inspect implemented interface methods. This could create a return-type decision without accounting for all the relevant override relationships.

(5)
DYNAMIC_RESOLUTION causes the entire closure expression to be dynamically compiled.
That gives other type-checking extensions or AST transformations no chance. They may want a particular expression to resolve dynamically without necessarily wanting to switch the compilation mode of the entire generated closure.

(6)
The new LazyQueryable.defer method does not have a GroovyABI annotation. The class is also public and has no @internal annotation, despite being described in STRATEGY.md as an implementation detail of the compiler integration. The positioning is not quite clear.

(7)
The extension registration and custom extensions
GinqStaticBridge.addExtension() modifies the existing annotation's extensions memberand handles a constant expression and a list expression, which covers the usual source forms. But, it silently does nothing when the existing expression has another form.
I am unsure of if there are extension-list expressions /produced through more complicated annotation processing.

(8)

def numbers = [1, 2, 3]
def query = Ginq.from(numbers).where { it > 1 }
def first = query.toList()
numbers.add(4)
def second = query.toList()
assert first != second // will fail?

While documented, this might still be surprising, but that is how the materialization idea works.

def query = GQ { ... }
def s1 = query.stream()
def s2 = query.stream()
assert s1 == s2
s1.count()
s2.count()  // <- stream already consumed?
query.toList()
def s3 = query.stream()
assert s3 != s1
s3.count()
def s4 = query.stream()
assert s4 != s3
s4.count()

Without a materialization stream() will return the same object, while after materialization it based off the list and a new stream is created every time.

[daniellansun]

The PR now targets master.

I ran the cases from the review. Notes on each, in the same order.

(1) Confirmed. @CompileDynamic and @CompileStatic(SKIP) skip the static-compilation visitor only. A @TypeChecked class still type-checks that method. @TypeChecked(SKIP) is the annotation that skips both visitors.

checked() was treating every skip as “do not adapt this method”, so the @TypeChecked class above saw the lowered query with no row-check extension. It now follows isSkipMode: the type-checked visitor looks only at @TypeChecked, and the static-compilation visitor looks at @TypeChecked and @CompileStatic. On a @TypeChecked class, a @CompileDynamic or @CompileStatic(SKIP) method gets the extension and the query wrapper. On a @CompileStatic class, that method is not type-checked, and the wrapper is not installed. A method with its own non-skip @TypeChecked is still checked.

(2) Confirmed for the two methods in the review. The extension has to be registered on the class annotation, because that is the member the checker reads, but the hooks no longer treat the rest of the class as a query.

value.mispelledProperty and value[0] on an enclosing-method parameter are compile errors again. Inside the query, an unknown row property stays dynamic: r.vlue on a list of maps is a missing key, not a type error. Outside the query the only receivers treated as rows are the query result itself (result[0].toList(), get / getAt on that result). A field and a parameter of the enclosing method are not.

(3) The snippet with int threshold = 5 inside the query does not compile, with or without the bridge. VariableScopeVisitor rejects a local that is already declared in an enclosing scope, including a closure parameter that reuses a local name. I did not special-case that into something Groovy otherwise forbids.

What the bridge does copy is decided by that visitor, not by a set of names. The lowered query is spliced into the method, the visitor links each reference to its declaration, and the original call is put back. A read-only local declared outside the query is snapshotted when the expression runs. A local the query assigns is shared, so the write is visible to the caller. A DynamicVariable is not copied: at conversion that is what an import alias still is, and the alias then resolves to the class. A field is read when the closure runs, not copied. STRATEGY.md matches this. The old name walk is not still in the bridge.

The snapshot is taken only when the result is deferred. @GQ(List) and GQL run at the call, and a later assignment is not something those queries observe.

(4) Confirmed. Widening was not limited to a real override.

Only a def return is widened. An explicit return type is left alone. The override check requires the same name, the same static-ness, and ParameterUtils.parametersEqual on the parameters. Private methods are ignored. Superclasses and getAllInterfaces() are both walked. A super return of Object does not block. A super return the widened type does not extend or implement does, so String load(List) on an interface stays String and the verifier reports that conflict instead of a Queryable return the method never declared. A same-named private method, and a static method with different parameters, do not block the widening.

(5) The whole lowered query closure is still compiled dynamically. I tried not to do that.

makeDynamic on each operator is not enough for codegen. StaticTypesCallSiteWriter throws on an operator it cannot prove, and a row comparison is that operator. Rewriting the operators to ScriptBytecodeAdapter calls compiled the comparison and then broke orderby x in desc (in is not a predicate there), join conditions, and the parameter types of parallel clause lambdas. A static lambda also cannot capture a local of an enclosing closure, which is the snapshot array: the bytecode loads the outer class and casts it to GroovyObject.

So the query closure is marked DYNAMIC_RESOLUTION = Boolean.TRUE and StaticTypesClosureWriter emits it with the dynamic writer. The enclosing method stays static. Calls inside the closure are still makeDynamic, and the stand-in target is cleared only there, so the dynamic writer does not emit invokevirtual of a method that does not exist. An extension registered before GinqRowChecks still sees the calls the visitor delivers on the enclosing method (toList, the macro call, defer). It does not see where and select as those names. I would rather those stayed visible. The codegen above is why the closure is dynamic anyway.

(6) LazyQueryable is public because the generated bytecode calls defer and is not in its package. The class is @Internal. defer is @GroovyABI(since = "7.0.0"). Callers program to Queryable. The class is the thunk that builds the pipeline on first read and then caches it. Fluent Ginq.from does not use it.

(7) Confirmed. A member that is not a string, a class literal, or a list or array of those was left unchanged and the extension was dropped. That is now a compile error, and the member is left as written. A class literal is stored as the class name, because the checker loads strings. A list that itself contains a call is the error case, not a supported form.

(8) Ginq.from(list) was caching the materialized list, so the second toList() did not see numbers.add(4). It re-reads the iterable on every terminal now. where and select still only record the work. Ginq.from on a Stream reads that stream once and replays the buffer; a stream cannot be read twice.

stream() was returning the same one-shot Stream until something materialized the query, so the second count() failed. stream() now returns a new stream each time and does not pull while creating it. Elements already pulled are replayed from the start; the rest are pulled from the pipeline once. count() on a sized pipeline does not pull, which is ordinary Stream.count and is what keeps an exists subquery from running its select just because the groups are built. After the source has been read to the end, later stream() calls read the buffer. None of those streams is the instance a previous caller is holding.

[blackdrag]

(3) The snippet with int threshold = 5 inside the query does not compile, with or without the bridge. VariableScopeVisitor rejects a local that is already declared in an enclosing scope, including a closure parameter that reuses a local name. I did not special-case that into something Groovy otherwise forbids.

I think there is a misunderstanding. That was not supposed to be two local variables, that was supposed to be a field/property and a local variable.

(5) The whole lowered query closure is still compiled dynamically. I tried not to do that.

makeDynamic on each operator is not enough for codegen. StaticTypesCallSiteWriter throws on an operator it cannot prove, and a row comparison is that operator. Rewriting the operators to ScriptBytecodeAdapter calls compiled the comparison and then broke orderby x in desc (in is not a predicate there), join conditions, and the parameter types of parallel clause lambdas. A static lambda also cannot capture a local of an enclosing closure, which is the snapshot array: the bytecode loads the outer class and casts it to GroovyObject.

I think you misunderstood the point was not that the closure is dynamic, the point was that it forces anything withing to be dynamic. It may change what another transform intended

[daniellansun]

(3) The example I answered last time really was two locals, and VariableScopeVisitor rejects that with or without the bridge. What you meant is a field or a property and a local of the same name. That does compile, and the bridge does not treat them as one name.

The lowered query is spliced into the method, VariableScopeVisitor links each reference to the declaration in scope, and the original call is put back. A FieldNode or a PropertyNode is not copied. It is read when the closure runs. A local or a closure parameter declared inside the query is a different declaration, and that is the one the references in that scope use. A DynamicVariable is not copied either. At conversion that is what an import alias still is, including a lower-case alias such as import java.util.concurrent.TimeUnit as threshold, and the alias then resolves to the class.

I ran that shape under @CompileStatic. A field threshold of 10 is set to 4 after the GQ expression and before toList(). The where clause sees 4. A closure parameter threshold of 5 in the same query's select stays 5. A getter/setter property behaves the same way: the getter runs at the terminal, not when the expression is built. @TypeChecked does this too. GQL still materializes at the expression, so a later write of the field is not observed. A closure parameter of the same name as the field wins over the field. Two locals of the same name are still rejected. STRATEGY.md now says this explicitly. There is still no name walk in the bridge.

(5) You are right that I answered a different question. The problem is not that the GINQ operators are dynamic. DYNAMIC_RESOLUTION = Boolean.TRUE is set on the lowered query closure, and StaticTypesClosureWriter compiles that whole closure with the dynamic writer. Every expression in it is dynamic, including a call written in where or select. A target another transform already chose for that call is not what is emitted. The call is dispatched at runtime.

A concrete case: an extension's onMethodSelection forces choose(CharSequence) when the argument is a String. In the enclosing @CompileStatic method that is the method that runs. The same call inside select hits choose(String), because the runtime type wins. The same is true of any other call in the clause whose static target differs from the runtime dispatch.

I tried not to mark the closure. Clause parameters stayed dynamic, so a map row is not inferred as a bean. A call the checker could resolve kept its target, and only an operator whose target was a stand-in became a dynamic call. The read-only snapshot was passed into the result lambda as a parameter, because a static lambda still cannot capture a local of an enclosing closure: the bytecode loads the outer class and casts it to GroovyObject. orderby n in desc and the join tests in the static suite still passed, and the choose(CharSequence) target was honoured inside the query.

The rest of the dynamic query did not. On the groovy-ginq suite, 71 tests failed. where note(n), note being a method of the enclosing class, was emitted as invokevirtual of Object.note(), which does not exist. A join alias was resolved against QueryableCollection. A parallel join that returns rows dynamically returned an empty list. Group, window, aggregate, and hash-join scripts failed in the same way. Parallel submit also cannot take the snapshot as an extra parameter, because that lambda is a Supplier.

So the closure stays dynamic. The enclosing method does not. I have written that limitation into STRATEGY.md and the CompileStatic section of the user guide, including the choose(CharSequence) example. I would rather the clause bodies stayed on the static writer. I do not have a way to do that which still runs the queries that pass today.

[daniellansun]

(5) Follow-up to #3259 (comment).

The lowered query closure is no longer marked DYNAMIC_RESOLUTION. Under @CompileStatic it stays on the static writer. A call the checker selects, including a target another extension already chose, is the method that is emitted.

The choose(CharSequence) case is the one you pointed at. An extension's onMethodSelection forces choose(CharSequence) when the argument is a String. That target is now honoured inside select as well. The regression is ['seq', 'seq'].

A well-typed Stream call in a clause is also the static method the checker selects, including a legal method-reference SAM. A number comparison the checker can prove stays the operator, so n > 1 when n is Integer is a primitive compare, which is what javac emits for the same program.

Only precise holes stay dynamic: an unresolved call, an operator the checker cannot prove, and a comparison it cannot type. A list of maps is a normal GINQ source, so @CompileStatic from r in [[a:1]] where r.missing > 1 still reaches ScriptBytecodeAdapter. r.missing is an untyped map read (Object), and Object > int is not a Java comparison; javac rejects that program. A number compared with a number is not on that path.

@TypeChecked still does not statically compile the clause bodies. The checker does not infer the clause-lambda parameter types, so those calls stay dynamic. That is unchanged.

A few existing tests used typings javac would reject, for example reduce(BigDecimal.ZERO, BigDecimal::add) on a stream of ints. Those were corrected in the smallest way that keeps the expected results, by mapping through BigDecimal.valueOf first. Collectors.joining() on Stream<String> was already well-typed and was left alone.

Queryable.from(int[]) now boxes to Queryable<Integer>. The type system no longer constructs Queryable<int> or Serializable<T>. SourceRecord has to stay public because navigate returns it and tests in another package name it; it is annotated @Internal and is not a published API.

ErrorCollector.hasErrors() now treats an empty list as no errors. A listener can remove a handled diagnostic; if that was the last one, the compile no longer fails with a blank startup failed.

STRATEGY.md and the CompileStatic section of the user guide are updated to match this.

[blackdrag]

that will potentially mean a lot of lock calls

[daniellansun]

Elements already in the replay buffer are served under the read lock. The callback runs after that lock is released. The write lock is taken only when the cursor is at the end of the buffer and the next element has to be pulled from upstream. Two concurrent readers of one stream see the same sequence, including nulls, with no lost or duplicated elements.

[blackdrag]

what happens for String<Integer>?

[daniellansun]

Thanks for the question. On the current tip (c56d922), a source use site such as String<Integer> is a compile error again, the same as in 6.0.0:

The class java.lang.String<java.lang.Integer> (supplied with 1 type parameter) refers to the class java.lang.String which takes no parameters

The revision you commented on dropped type arguments in ResolveVisitor when the class declares no type parameters, so String<Integer> would have compiled as a raw String. That was a mistake on my side, and c56d922 removes the rewriting. The arguments now reach GenericsVisitor, which rejects a use site that is not a well-formed parameterized type (JLS 4.5: it is an error if the class is not generic or the number of type arguments differs from its type parameters). ClosureReturnTypeSignatureTest.nonGenericUseSiteIsRejected covers this for java.io.Serializable and for a source class.

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: c56d922
▶️ Tests: 132073 executed
⚪️ Checks: 26/26 completed

---

Learn more about TestLens at testlens.app/docs.

[daniellansun]

@blackdrag @paulk-asert Thank you both for the earlier reviews. The current tip is c56d922, and CI is green.

The notes so far have replies on this tip, including the String<Integer> use site, which is a compile error again. I would be grateful if you could take another look when you have time.