feat(aggregate): support product aggregate function - #320
Merged
Merged
Conversation
Port FieldProductAgg from Java so that fields.<field-name>.aggregate-function=product no longer fails with "Use unsupported aggregation product". Agg multiplies and Retract divides, over TINYINT, SMALLINT, INT, BIGINT, FLOAT, DOUBLE and DECIMAL. Everything Java rejects is rejected here as an error rather than by wrapping around: an integer product or quotient outside the field type, an integer division by zero, and a decimal quotient with no finite decimal expansion, which is what Java's BigDecimal.divide() refuses. A decimal result the field precision can no longer hold aggregates to null, matching Decimal.fromBigDecimal. Decimal arithmetic runs through a 256 bit intermediate because the unscaled product of two decimal(38, 18) values does not fit into an int128, and rounds half up like Java's BigDecimal.setScale. Document the aggregation merge engine in the primary key table guide, which had no section for it, covering the options, the available functions and the type, null and decimal semantics of product.
SteNicholas
force-pushed
the
product-aggregate-function
branch
from
September 10, 2026 05:57
e535422 to
cbc7a18
Compare
Member
|
Thank you for the contribution and for improving the documentation! 👍 |
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.
Purpose
Linked issue: close #317
productis one of the aggregate functions Java Paimon supports, throughFieldProductAggandFieldProductAggFactory, butFieldAggregatorFactory::CreateFieldAggregatorhas no branch for it. A primary-key table configured with'merge-engine' = 'aggregation'and'fields.<field-name>.aggregate-function' = 'product'is therefore unusable: the aggregators for every value field are built as a group, so the whole merge function fails withInvalid: Use unsupported aggregation product or spell aggregate function incorrectly!as soon as the aggregation logic is initialized — eagerly inFileStoreWrite::Createon the write path, and on the first merge on the read path. That blocks writes, merge reads and compaction of tables a Java writer already produces.This PR ports
FieldProductAgg, following the shape of the existingFieldSumAgg: the per-type arithmetic is resolved once inCreate()into astd::function, so the merge path does not switch on the field type per row.Aggmultiplies the accumulator by the input andRetractdivides it, both keeping Java's null handling —Aggreturns whichever side is non-null, whileRetractreturns the accumulator unchanged when either side is null, so retracting into a null accumulator stays null rather than producing a reciprocal the waysumproduces a negation.Supported types are Java's
NUMERICfamily: TINYINT, SMALLINT, INT, BIGINT, FLOAT, DOUBLE and DECIMAL. Anything else is rejected byCreate(), matching thecheckArgumentinFieldProductAggFactory.__builtin_mul_overflowcatches products that leave the field type, and division checks for a zero divisor and forMIN / -1; each returnsStatus::Invalidat the points where Java throwsArithmeticException. Every width computes in its own type, so300 * 200overflows SMALLINT instead of silently widening. TINYINT is read back throughint8_tfirst, because the variant holds it as a plaincharwhose signedness follows the ABI.arrow::BasicDecimal256. A 128-bit intermediate is not enough: the unscaled product of twoDECIMAL(38, 18)values reaches 1e76, well pastint128. Multiplying doubles the scale, so the result is brought back down withReduceScaleBy(scale, round=true), which rounds away from zero on ties and therefore matches Java'sBigDecimal.setScale(scale, HALF_UP). Division cancels the scale out, so the dividend is scaled up first and the truncated quotient is then rounded half up from the remainder.Java's two decimal edge cases are reproduced rather than smoothed over, so a table reads the same way from either engine:
Decimal.fromBigDecimalreturns for it.BigDecimal.dividewithout a rounding mode throws there. This is narrower than rejecting every inexact division:1.00 / 8.00terminates at0.125and is still rounded half up to0.13, while2.00 / 3.00repeats and is refused. The test is whether the divisor, stripped of its factors of 2 and 5, divides the dividend — so3.00 / 6.00is accepted and a zero dividend always is.Changes:
field_product_agg.h/.cpp(new):FieldProductAgg, plus the file-local helpers it needs —ToDecimal256/ToInt128to move betweenpaimon::Decimalandarrow::BasicDecimal256,MultiplyExact/DivideExactfor the checked integer arithmetic, andQuotientTerminatesfor the finite-expansion test.field_aggregator_factory.h: registerproductnext tosum.src/paimon/CMakeLists.txt: add the source and the test to their targets.Cross-checked against Java: the expectations in
ProductAggregationITCasereproduce exactly, includingDECIMAL(5, 3)1.01 * 1.10 * 10.00 = 11.110andDECIMAL(4, 2)1.01 * 1.10 * 10.00 = 11.10where the intermediate rounds half up, and the product cases inFieldAggregatorTest(agg(null, 10) == 10,agg(1, 10) == 10,retract(10, 5) == 2,retract(null, 5) == null, and the overflow cases for all four integer widths).Tests
New UT in
field_product_agg_test.cpp:TestSimple,TestNull,TestSupportedTypes— multiply and divide over every accepted type, and the null combinations for both directions.TestIntegerRetractTruncatesTowardsZero— negative divisor, and10 / 3/-10 / 3truncating towards zero rather than rounding or flooring.TestDecimalRoundsHalfUp— ties, below-tie, negative operands, a negative divisor, and the case where the truncated quotient is zero so the rounding step must take its sign from the operands.TestDecimalRejectsNonTerminatingQuotient—2.00 / 3.00,1.00 / 7.00,0.01 / 1.50rejected;3.00 / 6.00and0.00 / 3.00accepted.TestDecimalWithZeroScale—DECIMAL(10, 0), where the rescale steps are no-ops.TestDecimalWiderThanInt128Intermediate—DECIMAL(38, 18), whose unscaled intermediate does not fit an int128.TestIntegerArithmeticErrors— overflow,MIN / -1and division by zero for all four integer widths.TestDecimalOverflowsToNull,TestDecimalDivisionByZero,TestFloatingPointDivisionByZero(signed infinities including a negative-zero divisor, andNaNfor0 / 0),TestInvalidType.Also
FieldAggregatorFactoryTest.TestSimplegains aproductcase, andAggregateMergeFunctionTest.TestProductcovers a multi-row merge (2 * 3 * 5) plus aDELETEretraction through the merge function.New IT in
write_and_read_inte_test.cpp:TestPKProductAggMergesAndSurvivesCompactionwrites three commits with overlapping keys over an INT and aDECIMAL(10, 2)column, verifies the merge-on-read result, then full-compacts and verifies the same result. The decimal values are chosen so every intermediate product is exact at scale 2, since decimal rounding is not associative and the merge order differs between merge-on-read and compaction.API and Format
No. No header under
include/paimon/is touched, and neither the storage format nor the protocol changes.productis already a legal value of the existingfields.<field-name>.aggregate-functionoption; it simply had no implementation behind it.Documentation
Yes.
docs/source/user_guide/primary_key_table.rsthad no merge engine section at all, only a passing mention of "the user-specified merge engine" under LSM trees. This PR adds a Merge Engines section with an Aggregation subsection covering the options, the fallback order, the available function names and retraction handling, and a product subsection covering the accepted types, the null semantics, integer exactness and truncation, IEEE 754 behaviour for FLOAT and DOUBLE, and the two DECIMAL boundary behaviours above.The other aggregate functions are listed by name only. Documenting their semantics was left out deliberately rather than written from their names, since this change did not verify them.
Generative AI tooling
Generated-by: Claude Code (claude-opus-5)