SteNicholas opened a new issue, #317: URL: https://github.com/apache/paimon-cpp/issues/317
## Search before asking - [x] I searched in the [issues](https://github.com/apache/paimon-cpp/issues) and found nothing similar. ## Motivation `product` is one of the aggregate functions Java Paimon supports, through `FieldProductAgg` and `FieldProductAggFactory`, but `FieldAggregatorFactory::CreateFieldAggregator` has no branch for it. A primary-key table configured with `'merge-engine' = 'aggregation'` and `'fields.<field>.aggregate-function' = 'product'` therefore cannot be opened at all by paimon-cpp: `AggregateMergeFunction::Create` builds one aggregator per value field up front, so the whole merge function fails with `Invalid: Use unsupported aggregation product or spell aggregate function incorrectly!` before a single row is read. That blocks both merge reads and compaction of tables a Java writer already produces. ## Solution Port `FieldProductAgg`, following the shape of the existing `FieldSumAgg`: resolve the per-type arithmetic once in `Create()` into a `std::function`, so the merge path does not switch on the field type per row. `Agg` multiplies the accumulator by the input and `Retract` divides it, both keeping Java's null handling — `Agg` returns whichever side is non-null, while `Retract` returns the accumulator unchanged when either side is null, so retracting into a null accumulator stays null rather than producing a reciprocal the way `sum` produces a negation. The supported types are Java's `NUMERIC` family: TINYINT, SMALLINT, INT, BIGINT, FLOAT, DOUBLE and DECIMAL. Anything else is rejected by `Create()`, matching the `checkArgument` in `FieldProductAggFactory`. - Integer arithmetic is exact. `__builtin_mul_overflow` catches products that leave the field type, and division checks for a zero divisor and for `MIN / -1`; each returns `Status::Invalid` at the points where Java throws `ArithmeticException`. Every width computes in its own type, so `300 * 200` overflows SMALLINT instead of silently widening. TINYINT is read back through `int8_t` first, because the variant holds it as a plain `char` whose signedness follows the ABI. - Decimal arithmetic runs through `arrow::BasicDecimal256`. A 128-bit intermediate is not enough: the unscaled product of two `DECIMAL(38, 18)` values reaches 1e76, well past `int128`. Multiplying doubles the scale, so the result is brought back down with `ReduceScaleBy(scale, round=true)`, which rounds away from zero on ties and therefore matches Java's `BigDecimal.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. Two decimal behaviors would differ from Java on purpose, and both are worth calling out before review: - A result that no longer fits the field precision returns an error. Java's `Decimal.fromBigDecimal` returns `null` there, so the column silently becomes NULL. Failing loudly loses less than silently dropping the value, and it keeps the decimal path consistent with how the integer paths report overflow. - Retraction rounds a quotient that has no finite decimal expansion half up. Java's `BigDecimal.divide` without a rounding mode throws instead. Wherever Java succeeds, both produce the same value. ## Anything else? No public API (`include/paimon/`), storage format, or option change. `product` is already a legal value of the existing `fields.<field>.aggregate-function` option, it simply had no implementation behind it. The change adds `field_product_agg.{h,cpp}` under `src/paimon/core/mergetree/compact/aggregate/`, registers the name in `FieldAggregatorFactory`, and wires both into `src/paimon/CMakeLists.txt`. Cross-checked against Java: the expectations in `ProductAggregationITCase` reproduce exactly, including `DECIMAL(5, 3)` `1.01 * 1.10 * 10.00 = 11.110` and `DECIMAL(4, 2)` `1.01 * 1.10 * 10.00 = 11.10`, where the intermediate rounds half up; likewise the product cases in `FieldAggregatorTest` (`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). ## Are you willing to submit a PR? - [x] I'm willing to submit a PR! -- 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]
