Skip to content

refact: Refactor Literal operator== to delegate to CompareTo for consistent equality semantics#313

Merged
lxy-9602 merged 6 commits into
alibaba:mainfrom
lxy-9602:fix-literal
May 29, 2026
Merged

refact: Refactor Literal operator== to delegate to CompareTo for consistent equality semantics#313
lxy-9602 merged 6 commits into
alibaba:mainfrom
lxy-9602:fix-literal

Conversation

@lxy-9602
Copy link
Copy Markdown
Collaborator

Purpose

No Linked issue.

The previous Literal::operator== had an independent implementation that used epsilon-based comparison for FLOAT/DOUBLE and raw byte comparison for other types. This was inconsistent with CompareTo() which uses Java-compatible total ordering for floats (CompareFloatingPoint: NaN == NaN, -0.0 < +0.0) and Decimal::CompareTo() for decimals. Having two different equality semantics was confusing and error-prone.

Now, operator== uses CompareTo() which provides numeric equality (e.g. decimals with different scales can compare equal, floats use total ordering).

Tests

API and Format

Documentation

Generative AI tooling

@zjw1111
Copy link
Copy Markdown
Collaborator

zjw1111 commented May 28, 2026

I found one issue that seems worth fixing before merging.

After this change, Literal::operator== follows CompareTo() == 0, so two decimal literals with different physical representations can now compare equal, for example 1 and 1.0 with different scales. However, Literal::HashCode() still hashes the decimal raw bits plus Scale(), and std::hash<Literal> directly returns that value.

That breaks the usual C++ hash contract: if a == b, then std::hash<Literal>{}(a) and std::hash<Literal>{}(b) should be equal. Otherwise, Literal can behave incorrectly in unordered_set, unordered_map, or any internal path that relies on HashCode().

Could we either keep equality representation-based, or update HashCode() to use the same normalized semantics as CompareTo()? It would also be good to add regression coverage for decimals with different scales that compare equal.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@lxy-9602
Copy link
Copy Markdown
Collaborator Author

@zjw1111 I left a note on this point. In Paimon’s real usage, the column schema guarantees that the precision and scale of a decimal are fixed, so decimals with different scales will not be inserted into the same unordered_map. As a result, this hash/equality mismatch does not occur in practice. We will also revisit the design more carefully going forward.

@lxy-9602 lxy-9602 merged commit 2c4019e into alibaba:main May 29, 2026
8 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.

4 participants