fix(spec): accept the legacy first_not_null_value aggregate name - #794
Open
jackylee-ch wants to merge 1 commit into
Open
fix(spec): accept the legacy first_not_null_value aggregate name#794jackylee-ch wants to merge 1 commit into
jackylee-ch wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Java registers
first_not_null_valueas an SPI alias offirst_non_null_value:FieldFirstNonNullValueAggLegacyFactorysits next to the canonical factory inMETA-INF/services/org.apache.paimon.factories.Factoryand builds the sameaggregator. It is deliberate rather than residue — it survives the switch-case to
SPI refactor in #4335 and
FirstValueAggregationITCasestill creates a table withit — but undocumented, so nothing advertises it.
Rust rejected the name at all three sites that key off it, so a table written by
Java or Flink planned fine and then failed once rows were pulled, with
ConfigInvalid: Unknown aggregate function 'first_not_null_value'. CREATE andALTER rejected it outright. The write path never checks the name, which is why
this only surfaces where the merge function is built.
Resolved through a single canonicalization point rather than one arm per site: the
validator/constructor guard test iterates a hardcoded name list, so a one-sided
edit would slip past it. Matching is exact, mirroring
FactoryUtil#discoverFactory,which compares identifiers with
equals.The alias stays out of the
supported: ...hint — Java does not document it, so itis read but never suggested — and errors still echo whatever the user wrote.
FieldAggregator::nameis&'static str, so the aggregator reports the canonicalname. That costs no parity: the only Java message repeating the configured
identifier is the retract rejection, and retract is refused here before an
aggregator exists.