Skip to content

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

Open
LuciferYang wants to merge 2 commits into
apache:masterfrom
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:masterfrom
LuciferYang:fix/predicate-json-literal-roundtrip

Conversation

@LuciferYang

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 literals. Nothing that worked before breaks, because reading them back never worked; but a non-Paimon REST server that parses the filter JSON itself would see a string where it previously saw an array or a number, so it is worth calling out in 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: those shapes have never been readable, 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.

Against the unfixed code it 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.

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.

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

2 participants