Skip to content

Navigation Menu

Sign in
Sign up

[common] Fix JSON round-trip of temporal and decimal predicate literals - #9640

Open
LuciferYang wants to merge 2 commits into
apache:master from
LuciferYang:fix/predicate-json-literal-roundtrip
Open

[common] Fix JSON round-trip of temporal and decimal predicate literals #9640
LuciferYang wants to merge 2 commits into
apache:master from
LuciferYang:fix/predicate-json-literal-roundtrip

Conversation

@LuciferYang

@LuciferYang LuciferYang commented Sep 5, 2026
edited
Loading

Copy link
Copy Markdown
Contributor

Purpose

close #9639

LeafPredicate handed its literals to Jackson as whatever PredicateBuilder.convertToJavaObject produced. JsonSerdeUtil registers JavaTimeModule without disabling WRITE_DATES_AS_TIMESTAMPS, so a LocalDate was written as [2026,1,15] and a BigDecimal as a JSON number; reading those back yields an ArrayList and a Double, and PredicateBuilder.convertJavaObject rejects both. DATE, TIME, TIMESTAMP, TIMESTAMP_LTZ and DECIMAL literals therefore could not round-trip at all: the writer's output is exactly what the reader refuses.

Those five now go out as strings, ISO-8601 for the temporal types and toPlainString for decimals, and come back through the matching parse before convertJavaObject sees them. Everything else is untouched.

This changes the wire representation of those five literals. For Paimon's own Java reader nothing that worked before breaks, because reading these literals back never worked: the writer emitted exactly what the reader refused. A REST server that parses the filter JSON with its own implementation is a separate case. Where it used to see an array or a number it now sees a string, and I have no evidence about how any given server handled the old forms. So the rollout order matters: the server has to understand the new string encoding before an upgraded client emits it. Deploy the server-side parser first, and call the encoding change out in the release notes.

The reader deliberately does not also accept the old array and number forms. I had that in an earlier version and dropped it: Paimon's reader never accepted those shapes, so there is no stored or in-flight predicate in that form that used to work, and reconstructing them means guessing at things like which trailing fields Jackson omitted and how much precision a double kept. If a concrete compatibility case turns up, that is worth adding on its own terms.

Tests

PredicateJsonSerdeTest.testTemporalAndDecimalLiteralsRoundTrip builds a predicate over a row with all five affected types, including a DECIMAL(20,3) with more significant digits than a double can hold, serializes it and asserts the parsed predicate equals the original.

RESTApiJsonTest covers the same five literals inside the actual ListPartitionsByFilterRequest envelope, so the REST request that carries a partition filter round-trips end to end.

Against the unfixed code the round-trip test fails with UnsupportedOperationException: Unexpected date literal of class java.util.ArrayList.

mvn -pl paimon-common -Dtest=PredicateJsonSerdeTest test on JDK 8: 91 tests, 0 failures. spotless:check and checkstyle:check on paimon-common are clean.

Jackson's JavaTimeModule wrote LocalDate/LocalTime/LocalDateTime/Instant
literals as arrays and BigDecimal as a JSON number, and
PredicateBuilder.convertJavaObject rejects the ArrayList and Double it
gets back, so DATE, TIME, TIMESTAMP, TIMESTAMP_LTZ and DECIMAL literals
could not survive the predicate JSON that the REST catalog sends and
reads.
Write them as ISO-8601 / plain strings and parse them back.

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The JSON literal fix has a real consumer: RESTCatalog.listPartitionsByFilterPaged serializes the Predicate into the REST request. I ran PredicateJsonSerdeTest on isolated base/head classpaths on JDK 8: the new temporal/decimal case fails on the base with an ArrayList date literal, and all 91 cases pass with the changed LeafPredicate.

Because this changes the REST wire representation, please document the string encoding and add a request-level compatibility test for temporal and high-precision decimal partition filters, including the intended behavior when client/server versions differ. The previous Java reader rejecting these forms does not by itself establish compatibility with servers that interpret the JSON independently. I did not find a defect in the local round-trip implementation; mixed-version REST behavior remains unverified.

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requirement fit: SUPPORTED. Implementation: CLEAN for the same-version path.

The new request-level test covers the actual ListPartitionsByFilterRequest envelope and preserves the temporal/high-precision decimal predicate after parsing its filter. PredicateJsonSerdeTest plus this request test passed 92 cases with the changed classes on JDK 8. No new local round-trip defect found.

One part of the earlier compatibility request remains open: this test uses the new LeafPredicate reader on both sides, so it does not establish mixed-version or independently implemented REST-server compatibility. The old Java reader rejects these new strings too. Please document the required server-side support/rollout order and avoid claiming that no server could read the old forms without evidence about those implementations. The string encoding itself is reasonable; deploy the matching parser before sending these literals from upgraded clients.

Copy link
Copy Markdown
Contributor Author

Updated the description to cover the open point. It now spells out the rollout order (a REST server has to understand the new string encoding before an upgraded client emits it, so deploy the server-side parser first and call it out in the release notes) and scopes the "nothing breaks" claim to Paimon's own Java reader, without asserting how independent servers treated the old array/number forms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JingsongLi JingsongLi JingsongLi left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] DATE, TIME, TIMESTAMP and DECIMAL predicate literals cannot survive predicate JSON

AltStyle によって変換されたページ (->オリジナル) /