Skip to content

[python] Align FieldSumAgg with Java implementation#8719

Merged
JingsongLi merged 19 commits into
apache:masterfrom
PyRSA:feature/pypaimon-sum-agg-impl
Jul 26, 2026
Merged

[python] Align FieldSumAgg with Java implementation#8719
JingsongLi merged 19 commits into
apache:masterfrom
PyRSA:feature/pypaimon-sum-agg-impl

Conversation

@PyRSA

@PyRSA PyRSA commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Purpose

This PR aligns the Python FieldSumAgg implementation with the Java implementation.

The changes include:

  • Support aggregation and retraction for all numeric types.
  • Add overflow checks for integral types (TINYINT, SMALLINT, INT, BIGINT).
  • Support DECIMAL aggregation with precision and scale handling.
  • Add comprehensive unit tests covering normal and overflow cases.

Dependency

This PR depends on #8718, which introduces the Python Decimal implementation and the numeric boundary constants used for overflow checking.

Tests

  • python -m pytest pypaimon/tests/test_field_aggregators.py::FieldSumAggTest -q
  • python -m pytest pypaimon/tests/test_aggregation_e2e.py -q
  • python -m pytest pypaimon/tests/test_aggregation_merge_function.py -q
  • git diff --check

@JingsongLi

Copy link
Copy Markdown
Contributor

This is also affected by the shared Decimal implementation; a valid DECIMAL(38,0) sum operation will fail due to precision=28, and encoding/decoding of unscaled bytes with scale>18 will also fail.

@PyRSA

PyRSA commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

This is also affected by the shared Decimal implementation; a valid DECIMAL(38,0) sum operation will fail due to precision=28, and encoding/decoding of unscaled bytes with scale>18 will also fail.

Fixed. This is resolved by the updates in the shared Decimal implementation (depends on #8718).

@JingsongLi

Copy link
Copy Markdown
Contributor

Please resolve conflicts.

@PyRSA

PyRSA commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Please resolve conflicts.

Resolved, thanks!

@JingsongLi

Copy link
Copy Markdown
Contributor

FieldSumAgg first performs Decimal addition outside the high-precision context, causing a valid DECIMAL(38,0) value to be truncated by Python's default 28-bit precision. The issue still occurs in the latest head: 12345678901234567890123456789012345678 + 1 actually returns 12345678901234567890123456790000000000, when the correct result should be .. .345679

@PyRSA

PyRSA commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

FieldSumAgg first performs Decimal addition outside the high-precision context, causing a valid DECIMAL(38,0) value to be truncated by Python's default 28-bit precision. The issue still occurs in the latest head: 12345678901234567890123456789012345678 + 1 actually returns 12345678901234567890123456790000000000, when the correct result should be .. .345679

Thanks. Fixed!

@JingsongLi

Copy link
Copy Markdown
Contributor

Same to #8718

@PyRSA

PyRSA commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Same to #8718

Thanks! This PR reuses the Decimal fixes from #8718, so #8718 needs to be merged first.

@JingsongLi

Copy link
Copy Markdown
Contributor

+1

@JingsongLi
JingsongLi merged commit bce7744 into apache:master Jul 26, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants