Skip to content

Conversation

HeartLinked
Copy link
Contributor

@HeartLinked HeartLinked commented Aug 20, 2025

Summary

Implements binary serialization and deserialization support for Literal values, enabling conversion between Literal objects and binary representations. Adds comprehensive formatting support for date, time, and timestamp types.

Changes

  • Added Conversions utility class (src/iceberg/util/conversions.cc/h) with ToBytes() and FromBytes() methods for Literal binary serialization/deserialization
  • Added literal formatting utilities (src/iceberg/util/literal_format.cc/h) for date, time, timestamp, and timestamptz formatting
  • Implemented Literal serialization methods: Replaced placeholder implementations of Serialize() and Deserialize() with full functionality
  • Enhanced Literal::ToString(): Added support for date, time, timestamp, and timestamptz types
  • Added TypeId string conversion: Implemented ToString(TypeId) utility function for type name lookups
  • Updated CMake configuration: Added new util source files to build system

Test Plan

  • Comprehensive binary round-trip tests for all primitive types (boolean, int, long, float, double, string, binary)
  • Serialization correctness tests verify exact byte representations match expected formats
  • Date/time formatting tests ensure proper ISO 8601 compatible string output
  • Modify existing test(e.g. manifest_reader_test.cc) to use binary serialization.

@HeartLinked HeartLinked changed the title feat: implement Literal class with serialization feat: implement literal expressions with binary serialization support Aug 21, 2025
@HeartLinked HeartLinked changed the title feat: implement literal expressions with binary serialization support feat: implement literal expressions with binary serialization support Aug 21, 2025
@HeartLinked HeartLinked force-pushed the feat/literal1 branch 2 times, most recently from b70c166 to 4084db1 Compare August 28, 2025 02:44
Copy link
Collaborator

@zhjwpku zhjwpku left a comment

Choose a reason for hiding this comment

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

LGTM

@HeartLinked
Copy link
Contributor Author

@wgtmac I've handled these comments.

@Fokko Fokko merged commit 046f149 into apache:main Oct 10, 2025
7 checks passed
@Fokko
Copy link
Contributor

Fokko commented Oct 10, 2025

Looks good @HeartLinked, thanks for working on this, and thanks @wgtmac @zhjwpku @mapleFU and @HuaHuaY for the review 🚀

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.

6 participants