xiangfu0 opened a new pull request, #19259: URL: https://github.com/apache/pinot/pull/19259
## Summary Adds automatic `ServiceLoader`-based discovery of external block-oriented `TransformFunction` implementations, so an application or plugin JAR can register transform functions without setting `pinot.server.transforms` or making application-specific server startup changes. Unlike `@ScalarFunction`s, external `TransformFunction` implementations previously could not self-register: `TransformFunctionFactory` only knew built-ins plus classes explicitly listed in `pinot.server.transforms`. Converting such functions to scalar functions is not an option when they need `ValueBlock`-level execution and batching. ## How it works An extension JAR ships a standard service descriptor: ``` META-INF/services/org.apache.pinot.core.operator.transform.function.TransformFunction ``` listing one implementation class per line, e.g.: ``` com.example.AiFilterTransformFunction com.example.AiClassifyTransformFunction com.example.AiGenerateTransformFunction ``` `TransformFunctionFactory.init(...)` (called by `ServerInstance` after plugins are loaded, before query traffic) discovers providers with `ServiceLoader.load(TransformFunction.class)` on the application classpath and from every classloader returned by `PluginManager.get().getPluginClassLoaders()` — the same pattern used by `PinotRuleSet` / `OperatorTypeRegistry`. Discovery never runs on the query path; `get()` performs plain lookups against an immutable snapshot. ### Provider contract A provider must be a public concrete class implementing `TransformFunction`, with a public no-argument constructor, returning a non-null/non-blank name from `getName()`. The startup-created instance is used only to obtain and validate the name and implementation class; it is never `init()`-ed or evaluated — query execution keeps constructing fresh instances through the factory. ### Registration and collision rules - Built-in registrations are always preserved; names are canonicalized with `TransformFunctionFactory.canonicalize()`. - The same implementation visible through overlapping application/plugin classloaders is de-duplicated; re-registering the identical implementation is a no-op. - Server startup **fails** (original cause preserved) if two different discovered classes claim the same canonical name, a discovered class collides with a built-in, a service descriptor is malformed, a provider cannot be constructed, or a provider returns a null/blank name. Collision errors include the function name, both implementation classes, and their classloaders. - `pinot.server.transforms` is preserved for backward compatibility and is applied after discovery, retaining its historical explicit-override semantics. Discovery skips classes that are also explicitly configured, so shipping a descriptor for an explicitly configured class (e.g. one that overrides a built-in name) never causes a collision failure. - A discovered name that matches an existing scalar function is registered with a WARN (not a failure): providing a block-oriented implementation of one's own scalar function is the same pattern the built-ins use, but the log flags potential semantic divergence (literal-only invocations still constant-fold through the scalar implementation at compile time). ### Initialization and thread safety `init(...)` is synchronized, builds and validates the registry in a local map, and atomically publishes an immutable snapshot to a `volatile` field. Readers never observe a partially initialized registry; repeated initialization is idempotent; a failed init leaves the previously published registry in place. There is no per-query or per-block discovery overhead. ### Scope This registers functions for server-side single-stage (leaf) execution only. It does not claim automatic MSE/Calcite registration — `PinotOperatorTable` support is a separate concern. No execution-semantics changes, no annotation-based classpath scanning, no static self-registration. ## Behavior change / release note - **New extension surface (always on, no flag):** server startup now scans the application classpath and every plugin classloader for `META-INF/services/org.apache.pinot.core.operator.transform.function.TransformFunction` descriptors. Deployments without such descriptors are completely unaffected — today no jar in the Pinot distribution ships one. - **New fail-fast startup mode:** a malformed descriptor, a provider that cannot be constructed, a null/blank function name, or a canonical-name collision now **fails server startup** (previously such descriptors were inert because nothing loaded them). This is deliberate: silently skipping a broken provider would surface later as `Unsupported function` at query time. Operators upgrading with third-party jars that happen to ship such a descriptor should validate them; there is intentionally no disable flag, matching the `OperatorTypeDescriptor`/`RuleSetCustomizer` discovery behavior. - **Rolling upgrade:** no wire-protocol, serialization, or config-format changes; roll-forward/roll-back safe. Explicit `pinot.server.transforms` semantics are unchanged. - The descriptor file name (`META-INF/services/org.apache.pinot.core.operator.transform.function.TransformFunction`) becomes a permanent external contract once released; a future move of `TransformFunction` into an SPI module would need a compatibility shim for the old descriptor name. - `getAllFunctions()` now returns an immutable snapshot current at call time (previously an unmodifiable live view); no in-repo production callers are affected, and the semantics are documented on the method. - Follow-ups (out of scope for this focused PR): extract a shared "context classloader + plugin classloaders, dedup by class name" ServiceLoader helper into pinot-spi and migrate `PinotRuleSet`/`OperatorTypeRegistry` to it; update the transform-function docs page to mention the `META-INF/services` registration path alongside `pinot.server.transforms` (pinot-docs PR). ## Testing - `TransformFunctionFactoryTest` (pinot-core, 21 tests): application-classpath discovery, discovery through a real `PluginManager` plugin realm, factory resolution + `ValueBlock` evaluation, case/underscore canonicalization, overlapping-classloader dedup, identical-class idempotency, discovered-vs-discovered and discovered-vs-built-in collisions, malformed descriptors, constructor failures, inaccessible constructors, null/blank names, built-ins preserved, legacy `pinot.server.transforms` registration + explicit overrides, no partial registry publication under concurrent reads, and confirmation that registration never calls `init()`/evaluation. Service descriptors are generated into temp dirs behind isolated `URLClassLoader`s so no fixture pollutes the test JVM or downstream test-jars. - `ServerInstanceTransformFunctionTest` (pinot-server, 3 tests): a service-provided transform registers through the (extracted) `ServerInstance.initTransformFunctions` production path without `pinot.server.transforms`; the legacy config path and its missing-class failure behavior still work. `spotless:check`, `checkstyle:check`, `license:check`, and `git diff --check` are clean on both modules. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
