[Sandbox] Add calcite-patched module: Gradle-driven patched Calcite build

[bowenlan-amzn]

Depends on #21549 publish patched calcite here

What

Wires the analytics-engine sandbox plugin to the pre-published patched Calcite at org.apache.calcite:calcite-core:1.41.0-opensearch-1 (and calcite-linq4j) from OpenSearch's S3 Maven snapshots repo (version composed from the calcite + calcite_os_rev keys in gradle/libs.versions.toml). No local Calcite build machinery required — it's just a Maven coordinate.

Why

Fixes CALCITE-3745 — Calcite hardcodes Janino's parent classloader, which breaks UDF resolution in OpenSearch's plugin classloader hierarchy. The patch introduces a TcclChainedClassLoader so Janino-compiled code can see classes in child plugin classloaders. Only runtime classloader behavior changes; API surface and bytecode references are identical to vanilla.

Design

• sandbox/build.gradle declares the OpenSearch Snapshots Maven repo and a resolutionStrategy.force on calcite-core / calcite-linq4j inside its existing subprojects { } block. Every sandbox subproject inherits both — so there's no need for per-plugin duplication. The force uses "${libVersions['calcite']}-opensearch-${libVersions['calcite_os_rev']}", reading the keys from gradle/libs.versions.toml as the single source of truth (bump calcite_os_rev there and every sandbox plugin follows). The force also pins unified-query-*'s transitive vanilla 1.41.0 (via calcite-babel) to the patched revision.
• analytics-framework declares api "org.apache.calcite:calcite-core:${calciteVersion}" (and calcite-linq4j), with calciteVersion composed from the same two libs.versions.toml keys. No local repo or force block — it inherits from the sandbox parent. The patched jars flow through runtimeClasspath of every consumer; the zip naturally ships the patched coordinate.
• License SHA-1 files under sandbox/libs/analytics-framework/licenses/ carry the -opensearch-1 coordinate:
  • calcite-core-1.41.0-opensearch-1.jar.sha1 → d4ac2aff0c76b2ea15f47940542999fa42e17d75
  • calcite-linq4j-1.41.0-opensearch-1.jar.sha1 → e9bcb0ec7ca38a4bff84283b39d4a736c5217645
• No post-processing of bundlePlugin. compileJava, test, forbiddenApis, thirdPartyAudit, dependencyLicenses, and the bundlePlugin Zip task all see the same calcite jars — the patched ones.

Companion

SQL plugin side: sql#5302 (sets the TCCL before invoking Calcite).

E2e verification

Reproduced the failure with vanilla Calcite, then confirmed the patched build fixes it. Query under test: source=t | eval u = upper(name) | fields u, which triggers SqlFunctions.<clinit> → Janino code-gen → parent-CL lookup — the exact path CALCITE-3745 is about.

Before / after (click to expand)

Companion branch for this run: sql#5403 (analytics-optional), rebased on latest feature/mustang-ppl-integration.

Before — vanilla Calcite in the zip

Zip ships with vanilla:

8811320 calcite-core-1.41.0.jar
 523019 calcite-linq4j-1.41.0.jar

Query returns HTTP 500:

{"error":{"reason":"There was internal problem at backend",
  "details":"Line 3, Column 3: Cannot determine simple type name \"org\"",
  "type":"CompileException"},"status":500}

Node log shows Janino failing to resolve the fully-qualified org.apache.calcite.runtime.SqlFunctions reference in its own generated code — classic CALCITE-3745: Janino's parent CL is pinned to a classloader that can't see the Calcite package it was generated from.

// Janino-generated body (excerpt)
return inputEnumerator.current() == null || inputEnumerator.current().toString() == null
    ? null
    : org.apache.calcite.runtime.SqlFunctions.upper(
        inputEnumerator.current() == null ? null : inputEnumerator.current().toString());

Caused by: org.codehaus.commons.compiler.CompileException:
    Line 3, Column 3: Cannot determine simple type name "org"
    at org.codehaus.janino.UnitCompiler.compileError(UnitCompiler.java:13228)
    at org.codehaus.janino.UnitCompiler.getRawReferenceType(UnitCompiler.java:7321)
    …
    at org.codehaus.janino.SimpleCompiler.cook(SimpleCompiler.java:219)
    at org.apache.calcite.adapter.enumerable.EnumerableInterpretable
        .compileToBindable(EnumerableInterpretable.java:176)

After — patched Calcite in the zip

Zip ships with patched:

8889483 calcite-core-1.41.0-opensearch-1.jar
 528940 calcite-linq4j-1.41.0-opensearch-1.jar

Same query, same cluster:

{"schema":[{"name":"u","type":"string"}],
 "datarows":[["ITEM1"],["ITEM2"]],
 "total":2,"size":2}

Log scan for CompileException / ClassNotFoundException.*calcite / Janino.*classloader across both nodes: 0 hits.

TcclChainedClassLoader (introduced by the patch) makes Janino's parent CL consult the thread context classloader first — which in the plugin CL hierarchy sees the Calcite package — before falling back to the original CL for anything else.

Check List

• Commits are signed per the DCO using --signoff.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit eebe687.

Path	Line	Severity	Description
sandbox/calcite-patched/build.gradle	85	high	Configuration-time bash script execution: the `if (wantsBuild)` block runs a ProcessBuilder shell script during Gradle configuration phase (before any task graph is built), not at task execution time. This executes arbitrary shell code whenever Gradle is configured for a broad set of task patterns, including common operations like `compileJava`. This is an unusual and high-risk pattern that bypasses normal Gradle task lifecycle controls.
sandbox/calcite-patched/build.gradle	143	high	Build-time external git clone from GitHub (https://github.com/apache/calcite.git) executed as part of the build process. Per mandatory supply chain rules, fetching and building external source code at build time must be flagged — the cloned repository's integrity cannot be verified by this diff, and a compromised upstream tag or MITM could introduce malicious code into the build.
sandbox/build.gradle	63	high	mavenLocal() added as a dependency repository for calcite-core and calcite-linq4j artifacts. Per mandatory supply chain rules, adding a package registry source must be flagged. The local Maven repository (~/.m2) can be poisoned by any process with filesystem access to the build host, making this a potential supply chain injection vector for the calcite artifacts consumed by all sandbox subprojects.
gradle/libs.versions.toml	108	high	New dependency version entries added: calcite=1.41.0 and calcite_opensearch_rev=1. Per mandatory supply chain rules, all dependency version changes must be flagged for maintainer verification. These entries define the locally-patched Calcite artifact coordinate consumed across multiple sandbox subprojects.
sandbox/plugins/analytics-engine/build.gradle	113	high	New runtime dependency added: org.apache.commons:commons-text:1.11.0. Per mandatory supply chain rules, all new package additions must be flagged for maintainer verification regardless of how legitimate the artifact name appears.
sandbox/calcite-patched/build.gradle	103	medium	The configuration-time ProcessBuilder copies the entire parent process environment via `env.putAll(System.getenv())` and passes it to the spawned bash subprocess. This propagates all environment variables — including CI secrets, tokens, and credentials present in the build environment — into the externally-cloned Calcite nested Gradle build, which could exfiltrate them if the cloned build scripts are malicious.

The table above displays the top 10 most important findings.

Total: 6 | Critical: 0 | High: 5 | Medium: 1 | Low: 0

---

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

---

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit e916936)

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to a397777

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Read TCCL dynamically at class-load time The anonymous ClassLoader captures tccl at the time chain() is called (a snapshot), rather than reading Thread.currentThread().getContextClassLoader() at class-load time. If the TCCL changes between the call to chain() and the actual Janino compilation (e.g., in a different thread or after a context switch), the chained loader will use a stale reference. Consider reading TCCL dynamically inside loadClass to always reflect the current thread's context. sandbox/calcite-patched/patches/0001-CALCITE-3745-prefer-TCCL-for-Janino-parent-classloader.patch [133-146] return new ClassLoader(fallback) { @Override protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException { - try { - Class<?> c = tccl.loadClass(name); - if (resolve) { - resolveClass(c); + ClassLoader currentTccl = Thread.currentThread().getContextClassLoader(); + if (currentTccl != null && currentTccl != getParent()) { + try { + Class<?> c = currentTccl.loadClass(name); + if (resolve) { + resolveClass(c); + } + return c; + } catch (ClassNotFoundException ignored) { } - return c; - } catch (ClassNotFoundException e) { - return super.loadClass(name, resolve); } + return super.loadClass(name, resolve); } }; Suggestion importance[1-10]: 6 __ Why: The suggestion is technically valid — capturing tccl at chain() call time rather than reading it dynamically inside loadClass could lead to stale references in multi-threaded scenarios. However, the improved code changes the semantics significantly (it no longer checks if currentTccl == fallback, only if it equals getParent()), and the original design may be intentional for performance or consistency reasons within a single compilation context.	Low

---

Previous suggestions

Suggestions up to commit a4126ad

Category	Suggestion	Impact
Possible issue	Read TCCL at load time, not capture time The anonymous ClassLoader captures tccl at the time chain() is called, but the thread context classloader should be read at class-loading time to reflect the TCCL of the thread that actually triggers Janino compilation. If chain() is called on a different thread than the one that later invokes loadClass, the captured TCCL may be stale or wrong. Read Thread.currentThread().getContextClassLoader() inside loadClass instead. sandbox/calcite-patched/patches/0001-CALCITE-3745-prefer-TCCL-for-Janino-parent-classloader.patch [133-147] return new ClassLoader(fallback) { @Override protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException { - try { - Class<?> c = tccl.loadClass(name); - if (resolve) { - resolveClass(c); + ClassLoader currentTccl = Thread.currentThread().getContextClassLoader(); + if (currentTccl != null && currentTccl != getParent()) { + try { + Class<?> c = currentTccl.loadClass(name); + if (resolve) { + resolveClass(c); + } + return c; + } catch (ClassNotFoundException ignored) { } - return c; - } catch (ClassNotFoundException e) { - return super.loadClass(name, resolve); } + return super.loadClass(name, resolve); } }; Suggestion importance[1-10]: 7 __ Why: This is a valid and meaningful concern: capturing tccl at chain() call time rather than at loadClass invocation time could lead to stale classloader references if the calling thread differs from the compilation thread. The improved code correctly reads TCCL dynamically inside loadClass, which better matches the intended semantics of TCCL-based resolution.	Medium
General	Clarify intentional JAVA_HOME override ordering When setup-java is called twice, the second invocation overwrites JAVA_HOME with JDK 25, which is the desired default for the outer build. However, steps.jdk17.outputs.path should still be valid. To make the intent explicit and avoid accidental breakage if action behavior changes, consider adding a verification step or a comment clarifying that JDK 25 intentionally overrides JAVA_HOME after JDK 17 is captured via its step output. .github/workflows/sandbox-check.yml [25-35] - name: Set up JDK 17 (for Calcite nested build) id: jdk17 uses: actions/setup-java@v5 with: java-version: 17 distribution: temurin +# JDK 25 is set up after JDK 17 so that JAVA_HOME points to JDK 25 for the +# outer OpenSearch build. The JDK 17 path is captured via steps.jdk17.outputs.path. - name: Set up JDK 25 uses: actions/setup-java@v5 with: java-version: 25 distribution: temurin cache: gradle Suggestion importance[1-10]: 2 __ Why: This suggestion only adds a comment to clarify the ordering of JDK setup steps, which is a minor documentation improvement. The improved_code adds a YAML comment but doesn't change any functional behavior, making this a low-impact suggestion.	Low

Suggestions up to commit 8708379

Category	Suggestion	Impact
Possible issue	Use dynamic TCCL lookup at class-load time The anonymous ClassLoader captures tccl at the time chain() is called (a snapshot), but the intent of TCCL is to reflect the current thread's context classloader at class-load time. If the TCCL changes between when chain() is invoked and when Janino actually loads a class, the wrong classloader will be used. The lookup should call Thread.currentThread().getContextClassLoader() inside loadClass instead of closing over the captured tccl. sandbox/calcite-patched/patches/0001-CALCITE-3745-prefer-TCCL-for-Janino-parent-classloader.patch [133-146] return new ClassLoader(fallback) { @Override protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException { - try { - Class<?> c = tccl.loadClass(name); - if (resolve) { - resolveClass(c); + ClassLoader currentTccl = Thread.currentThread().getContextClassLoader(); + if (currentTccl != null && currentTccl != getParent()) { + try { + Class<?> c = currentTccl.loadClass(name); + if (resolve) { + resolveClass(c); + } + return c; + } catch (ClassNotFoundException ignored) { } - return c; - } catch (ClassNotFoundException e) { - return super.loadClass(name, resolve); } + return super.loadClass(name, resolve); } }; Suggestion importance[1-10]: 6 __ Why: The suggestion raises a valid concern: capturing tccl at chain() call time means the classloader is a snapshot rather than dynamically resolved per-load. However, in practice Janino compiles code in a single-threaded context where the TCCL is set before calling chain() and remains stable, so the practical impact may be limited. The improved code is logically sound and would make the behavior more correct in edge cases.	Low
General	Add missing trailing newline to checksum file The SHA1 file is missing a trailing newline (the diff shows \ No newline at end of file). Many tools that verify checksums expect a newline-terminated file; its absence can cause checksum verification failures. Add a newline at the end of the file. sandbox/libs/analytics-framework/licenses/calcite-core-1.41.0.jar.sha1 [1] +0dd7b4be638f0cea174f78cc851322b64d813a1e - Suggestion importance[1-10]: 2 __ Why: The existing_code and improved_code are identical, and the diff doesn't clearly show a missing newline issue. The suggestion offers marginal value and the improved code doesn't demonstrate any actual change.	Low

Suggestions up to commit f731eed

Category	Suggestion	Impact
Possible issue	Use correct java-home output key for setup-java When setup-java is called twice, the second invocation overwrites JAVA_HOME and the java on PATH, which is the desired behavior for the outer build. However, the steps.jdk17.outputs.path value should be verified to be the correct output key for actions/setup-java@v5 — the correct output is java-home, not path. Using the wrong key will silently pass an empty string as -PcalciteJdk17Home. .github/workflows/sandbox-check.yml [25-35] - name: Set up JDK 17 (for Calcite nested build) id: jdk17 uses: actions/setup-java@v5 with: java-version: 17 distribution: temurin - name: Set up JDK 25 uses: actions/setup-java@v5 with: java-version: 25 distribution: temurin cache: gradle +# Then in the run step: +# -PcalciteJdk17Home=${{ steps.jdk17.outputs.java-home }} Suggestion importance[1-10]: 8 __ Why: The actions/setup-java@v5 action outputs java-home, not path. Using steps.jdk17.outputs.path would silently pass an empty string to -PcalciteJdk17Home, causing the Calcite nested build to fail without a clear error. The improved_code correctly identifies the fix as using steps.jdk17.outputs.java-home.	Medium
	Read TCCL at resolution time, not capture time The anonymous ClassLoader captures tccl at the time chain() is called, but the TCCL may change between the time the classloader is created and when it is actually used by Janino. The TCCL should be read inside loadClass (i.e., at class-resolution time) to reflect the current thread's context, which is the whole point of the TCCL workaround. sandbox/calcite-patched/patches/0001-CALCITE-3745-prefer-TCCL-for-Janino-parent-classloader.patch [133-147] return new ClassLoader(fallback) { @Override protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException { - try { - Class<?> c = tccl.loadClass(name); - if (resolve) { - resolveClass(c); + ClassLoader currentTccl = Thread.currentThread().getContextClassLoader(); + if (currentTccl != null && currentTccl != fallback) { + try { + Class<?> c = currentTccl.loadClass(name); + if (resolve) { + resolveClass(c); + } + return c; + } catch (ClassNotFoundException ignored) { } - return c; - } catch (ClassNotFoundException e) { - return super.loadClass(name, resolve); } + return super.loadClass(name, resolve); } }; Suggestion importance[1-10]: 7 __ Why: This is a valid and important correctness concern: capturing tccl at chain() call time means the classloader won't reflect TCCL changes that happen between creation and actual class resolution by Janino. Reading TCCL inside loadClass is the semantically correct approach for a TCCL-based workaround.	Medium

Suggestions up to commit 57eb4ce

Category	Suggestion	Impact
Possible issue	Capture TCCL at resolution time, not creation time The anonymous ClassLoader captures tccl at the time chain() is called (a snapshot), but the TCCL may change between the time the classloader is created and when loadClass is actually invoked by Janino. To correctly reflect the current TCCL at resolution time, tccl should be retrieved inside loadClass rather than captured from the enclosing scope. sandbox/calcite-patched/patches/0001-CALCITE-3745-prefer-TCCL-for-Janino-parent-classloader.patch [133-147] return new ClassLoader(fallback) { @Override protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException { - try { - Class<?> c = tccl.loadClass(name); - if (resolve) { - resolveClass(c); + ClassLoader currentTccl = Thread.currentThread().getContextClassLoader(); + if (currentTccl != null && currentTccl != getParent()) { + try { + Class<?> c = currentTccl.loadClass(name); + if (resolve) { + resolveClass(c); + } + return c; + } catch (ClassNotFoundException ignored) { } - return c; - } catch (ClassNotFoundException e) { - return super.loadClass(name, resolve); } + return super.loadClass(name, resolve); } }; Suggestion importance[1-10]: 6 __ Why: The suggestion is technically valid — capturing tccl at chain() call time means the classloader won't reflect TCCL changes that happen between creation and actual class resolution by Janino. However, in practice the TCCL is typically set before Janino compilation begins and remains stable, so this is a minor correctness concern rather than a critical bug.	Low
General	Add Gradle cache to JDK 17 setup step The JDK 17 setup step does not set cache: gradle, while the JDK 25 step does. Since the Calcite nested build uses JDK 17 and also invokes Gradle, omitting the cache here means the Calcite Gradle wrapper dependencies won't be cached, potentially slowing CI. Consider adding cache: gradle to the JDK 17 setup step as well, or at minimum document why it is intentionally omitted. .github/workflows/sandbox-check.yml [25-30] - name: Set up JDK 17 (for Calcite nested build) id: jdk17 uses: actions/setup-java@v5 with: java-version: 17 distribution: temurin + cache: gradle Suggestion importance[1-10]: 3 __ Why: Adding cache: gradle to the JDK 17 step could speed up CI by caching the Calcite nested build's Gradle dependencies, but this is a minor optimization. The outer build's JDK 25 step already handles the main Gradle cache, and the Calcite nested build may have a separate cache key anyway.	Low

Suggestions up to commit ab8fe61

Category	Suggestion	Impact
General	Capture TCCL dynamically at load time The anonymous ClassLoader captures tccl at the time chain() is called (a snapshot), rather than reading Thread.currentThread().getContextClassLoader() at each loadClass invocation. If the TCCL changes between the time the classloader is created and when Janino actually compiles code (which can happen in async or thread-pool scenarios), the captured TCCL will be stale and UDFs registered later won't be visible. Consider reading the TCCL dynamically inside loadClass to always reflect the current thread's context. sandbox/calcite-patched/patches/0001-CALCITE-3745-prefer-TCCL-for-Janino-parent-classloader.patch [133-147] return new ClassLoader(fallback) { @Override protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException { + ClassLoader current = Thread.currentThread().getContextClassLoader(); + if (current == null || current == fallback) { + return super.loadClass(name, resolve); + } try { - Class<?> c = tccl.loadClass(name); + Class<?> c = current.loadClass(name); if (resolve) { resolveClass(c); } return c; } catch (ClassNotFoundException e) { return super.loadClass(name, resolve); } } }; Suggestion importance[1-10]: 6 __ Why: The suggestion is technically valid — capturing tccl at chain() call time rather than dynamically at loadClass invocation could lead to stale TCCL in async/thread-pool scenarios. However, the patch is specifically designed for a known OpenSearch use case where TCCL is set before Janino compilation, so the practical impact may be limited. The improved code accurately reflects the suggested change.	Low
	Enable Gradle cache for nested JDK build The JDK 17 setup step does not enable Gradle caching, while the JDK 25 step does. Since both JDKs are used in the same workflow, the Gradle cache is only associated with the JDK 25 setup. This is likely fine for the outer build, but if the nested Calcite build (running under JDK 17) also uses Gradle's cache, it won't benefit from caching. Consider whether the JDK 17 step also needs cache: gradle or if this is intentional. .github/workflows/sandbox-check.yml [25-35] - name: Set up JDK 17 (for Calcite nested build) id: jdk17 uses: actions/setup-java@v5 with: java-version: 17 distribution: temurin + cache: gradle - name: Set up JDK 25 uses: actions/setup-java@v5 with: java-version: 25 distribution: temurin cache: gradle Suggestion importance[1-10]: 3 __ Why: The suggestion to add cache: gradle to the JDK 17 step is a minor optimization. The nested Calcite build uses a separate Gradle wrapper and may benefit from caching, but this is a marginal improvement and the suggestion itself notes it may be intentional to omit it.	Low

[github-actions]

❌ Gradle check result for 3ad6909: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit f8e0974

[github-actions]

❌ Gradle check result for f8e0974: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit d8f2baf

[github-actions]

❌ Gradle check result for d8f2baf: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 558479b

[github-actions]

Persistent review updated to latest commit f338687

[github-actions]

Persistent review updated to latest commit a0edff1

[github-actions]

Persistent review updated to latest commit df2de8b

[github-actions]

Persistent review updated to latest commit e8f5525

[github-actions]

Persistent review updated to latest commit 8378fc0

[github-actions]

✅ Gradle check result for 8378fc0: SUCCESS

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.53%. Comparing base (e0a06f2) to head (e916936).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #21501      +/-   ##
============================================
- Coverage     73.54%   73.53%   -0.02%     
- Complexity    74628    74633       +5     
============================================
  Files          5978     5978              
  Lines        338734   338734              
  Branches      48842    48842              
============================================
- Hits         249127   249084      -43     
- Misses        69784    69812      +28     
- Partials      19823    19838      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

Persistent review updated to latest commit 13b07cc

[github-actions]

✅ Gradle check result for 13b07cc: SUCCESS

[github-actions]

Persistent review updated to latest commit 0996b90

[github-actions]

❌ Gradle check result for 0996b90: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit e916936

[github-actions]

✅ Gradle check result for e916936: SUCCESS