dao-jun commented on PR #25967:
URL: https://github.com/apache/pulsar/pull/25967#issuecomment-4667421563
overall LGTM, just 2 points:
1. The FunctionCommon.getFunctionClassParent() change is more critical
than the PIP describes — it's not about "keeping type inference correct," it's
that without it, deployment will NPE (assuming doJavaChecks is fixed first).
The call chain is getFunctionTypes → getFunctionClassParent →
resolveInterfaceTypeArguments.
When the parent interface is misidentified as java.util.function.Function,
resolveInterfaceTypeArguments traverses the interface hierarchy and never finds
Function (since IncrementalWindowFunction doesn't extend it), returns null, and
typeArgsList.get(0) throws NPE. This should be documented as a hard dependency,
not a
correctness improvement.
2. Window.get() / getNew() / getExpired() return mutable List<T>. While
each trigger creates fresh lists so mutations can't corrupt internal state,
once this becomes a public API the Javadoc should explicitly state whether
callers are allowed to modify the returned lists.
--
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]
