You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "parthchandra (via GitHub)" <gi...@apache.org> on 2026/01/22 22:16:57 UTC
[PR] perf: [iceberg] Use protobuf instead of JSON to serialize Iceberg partition values [datafusion-comet]
parthchandra opened a new pull request, #3247:
URL: https://github.com/apache/datafusion-comet/pull/3247
## Rationale for this change
We see increased GC collection times in jobs with Iceberg scans with a large number (10K-100K) of partitions
## What changes are included in this PR?
Partition values are currently serialized to native by constructing a JSON string. This PR changes that to use Protobuf instead.
## How are these changes tested?
Added a new unit test for a large number of partitions.
AI note: large parts were generated using Claude Code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: [iceberg] Use protobuf instead of JSON to serialize Iceberg partition values [datafusion-comet]
Posted by "parthchandra (via GitHub)" <gi...@apache.org>.
parthchandra merged PR #3247:
URL: https://github.com/apache/datafusion-comet/pull/3247
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: [iceberg] Use protobuf instead of JSON to serialize Iceberg partition values [datafusion-comet]
Posted by "parthchandra (via GitHub)" <gi...@apache.org>.
parthchandra commented on PR #3247:
URL: https://github.com/apache/datafusion-comet/pull/3247#issuecomment-3792094606
Merged. Thanks @andygrove @hsiang-c
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: [iceberg] Use protobuf instead of JSON to serialize Iceberg partition values [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #3247:
URL: https://github.com/apache/datafusion-comet/pull/3247#issuecomment-3791945315
@parthchandra do you have benchmarks showing the performance improvement?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: [iceberg] Use protobuf instead of JSON to serialize Iceberg partition values [datafusion-comet]
Posted by "parthchandra (via GitHub)" <gi...@apache.org>.
parthchandra commented on PR #3247:
URL: https://github.com/apache/datafusion-comet/pull/3247#issuecomment-3792623463
> @parthchandra do you have benchmarks showing the performance improvement?
(Sorry, didn't notice this before merging).
This likely won't have a performance benefit as much as it might help with the GC pressure we see when serializing the plan involving a large number of partitions.
@hisang-c is helping with the profiling runs. We will post some data from the profiling runs here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: [iceberg] Use protobuf instead of JSON to serialize Iceberg partition values [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #3247:
URL: https://github.com/apache/datafusion-comet/pull/3247#discussion_r2718923074
##########
native/proto/src/proto/operator.proto:
##########
@@ -110,6 +110,32 @@ message NativeScan {
bool encryption_enabled = 14;
}
+// Partition value for Iceberg partition data
+message PartitionValue {
+ int32 field_id = 1;
+ oneof value {
+ int32 int_val = 2;
+ int64 long_val = 3;
+ int64 date_val = 4; // days since epoch
+ int64 timestamp_val = 5; // microseconds since epoch
+ int64 timestamp_tz_val = 6; // microseconds with timezone
+ string string_val = 7;
+ double double_val = 8;
+ float float_val = 9;
+ bytes decimal_val = 10; // unscaled BigInteger bytes
+ bool bool_val = 11;
+ bytes uuid_val = 12;
+ bytes fixed_val = 13;
+ bytes binary_val = 14;
Review Comment:
We may want to consider consolidating this with the existing `Literal` defined in protobuf. This does not need to happen for the current PR.
```protobuf
message Literal {
oneof value {
bool bool_val = 1;
// Protobuf doesn't provide int8 and int16, we put them into int32 and convert
// to int8 and int16 when deserializing.
int32 byte_val = 2;
int32 short_val = 3;
int32 int_val = 4;
int64 long_val = 5;
float float_val = 6;
double double_val = 7;
string string_val = 8;
bytes bytes_val = 9;
bytes decimal_val = 10;
ListLiteral list_val = 11;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: [iceberg] Use protobuf instead of JSON to serialize Iceberg partition values [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #3247:
URL: https://github.com/apache/datafusion-comet/pull/3247#issuecomment-3787256061
@parthchandra there is a clippy failure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: [iceberg] Use protobuf instead of JSON to serialize Iceberg partition values [datafusion-comet]
Posted by "parthchandra (via GitHub)" <gi...@apache.org>.
parthchandra commented on PR #3247:
URL: https://github.com/apache/datafusion-comet/pull/3247#issuecomment-3792832450
> > @parthchandra do you have benchmarks showing the performance improvement?
>
> (Sorry, didn't notice this before merging). This likely won't have a performance benefit as much as it might help with the GC pressure we see when serializing the plan involving a large number of partitions.
Didn't seem to have too much of an impact.
FilePartition info went from `2.96%` of total allocation to `2.94%`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: [iceberg] Use protobuf instead of JSON to serialize Iceberg partition values [datafusion-comet]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3247:
URL: https://github.com/apache/datafusion-comet/pull/3247#issuecomment-3787133289
## [Codecov](https://app.codecov.io/gh/apache/datafusion-comet/pull/3247?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `100 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 20.92%. Comparing base ([`f09f8af`](https://app.codecov.io/gh/apache/datafusion-comet/commit/f09f8af64c6599255e116a376f4f008f2fd63b43?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`2d18f57`](https://app.codecov.io/gh/apache/datafusion-comet/commit/2d18f57c9375a7a761f74afa060df675bbef86d9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 866 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion-comet/pull/3247?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [.../comet/serde/operator/CometIcebergNativeScan.scala](https://app.codecov.io/gh/apache/datafusion-comet/pull/3247?src=pr&el=tree&filepath=spark%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fcomet%2Fserde%2Foperator%2FCometIcebergNativeScan.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3Bhcmsvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9jb21ldC9zZXJkZS9vcGVyYXRvci9Db21ldEljZWJlcmdOYXRpdmVTY2FuLnNjYWxh) | 0.00% | [100 Missing :warning: ](https://app.codecov.io/gh/apache/datafusion-comet/pull/3247?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #3247 +/- ##
=============================================
- Coverage 56.12% 20.92% -35.21%
+ Complexity 976 472 -504
=============================================
Files 119 170 +51
Lines 11743 15858 +4115
Branches 2251 2638 +387
=============================================
- Hits 6591 3318 -3273
- Misses 4012 12006 +7994
+ Partials 1140 534 -606
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/datafusion-comet/pull/3247?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: [iceberg] Use protobuf instead of JSON to serialize Iceberg partition values [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #3247:
URL: https://github.com/apache/datafusion-comet/pull/3247#discussion_r2718916933
##########
native/core/src/execution/planner.rs:
##########
@@ -2589,6 +2650,105 @@ fn convert_spark_types_to_arrow_schema(
arrow_schema
}
+/// Converts a protobuf PartitionValue to an iceberg Literal.
+///
+/// This replaces JSON parsing with direct protobuf deserialization with a more compact
+/// representation (e.g., timestamps as integers vs strings).
Review Comment:
nit: we can remove the part of the comment that explains how this used to work
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: [iceberg] Use protobuf instead of JSON to serialize Iceberg partition values [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #3247:
URL: https://github.com/apache/datafusion-comet/pull/3247#discussion_r2719001036
##########
spark/src/main/scala/org/apache/comet/serde/operator/CometIcebergNativeScan.scala:
##########
@@ -71,14 +71,138 @@ object CometIcebergNativeScan extends CometOperatorSerde[CometBatchScanExec] wit
}
/**
- * Converts an Iceberg partition value to JSON format expected by iceberg-rust.
- *
- * iceberg-rust's Literal::try_from_json() expects specific formats for certain types:
- * - Timestamps: ISO string format "yyyy-MM-dd'T'HH:mm:ss.SSSSSS"
- * - Dates: ISO string format "YYYY-MM-DD"
- * - Decimals: String representation
- *
- * See: iceberg-rust/crates/iceberg/src/spec/values/literal.rs
+ * Converts an Iceberg partition value to protobuf format. Protobuf is less verbose than JSON.
+ * The following types are also serialized as integer values instead of as strings - Timestamps,
+ * Dates, Decimals, FieldIDs
+ */
+ private def partitionValueToProto(
+ fieldId: Int,
+ fieldTypeStr: String,
+ value: Any): OperatorOuterClass.PartitionValue = {
+ val builder = OperatorOuterClass.PartitionValue.newBuilder()
+ builder.setFieldId(fieldId)
+
+ if (value == null) {
+ builder.setIsNull(true)
+ } else {
+ builder.setIsNull(false)
+ fieldTypeStr match {
+ case t if t.startsWith("timestamp") =>
+ val micros = value match {
+ case l: java.lang.Long => l.longValue()
+ case i: java.lang.Integer => i.longValue()
+ case _ => value.toString.toLong
+ }
+ if (t.contains("tz")) {
+ builder.setTimestampTzVal(micros)
+ } else {
+ builder.setTimestampVal(micros)
+ }
+
+ case "date" =>
+ val days = value.asInstanceOf[java.lang.Integer].intValue()
+ builder.setDateVal(days)
+
+ case d if d.startsWith("decimal(") =>
+ // Serialize as unscaled BigInteger bytes
+ val bigDecimal = value match {
+ case bd: java.math.BigDecimal => bd
+ case _ => new java.math.BigDecimal(value.toString)
+ }
+ val unscaledBytes = bigDecimal.unscaledValue().toByteArray
+ builder.setDecimalVal(com.google.protobuf.ByteString.copyFrom(unscaledBytes))
+
+ case "string" =>
+ builder.setStringVal(value.toString)
+
+ case "int" =>
+ val intVal = value match {
+ case i: java.lang.Integer => i.intValue()
+ case l: java.lang.Long => l.intValue()
+ case _ => value.toString.toInt
+ }
+ builder.setIntVal(intVal)
+
+ case "long" =>
+ val longVal = value match {
+ case l: java.lang.Long => l.longValue()
+ case i: java.lang.Integer => i.longValue()
+ case _ => value.toString.toLong
+ }
+ builder.setLongVal(longVal)
+
+ case "float" =>
+ val floatVal = value match {
+ case f: java.lang.Float => f.floatValue()
+ case d: java.lang.Double => d.floatValue()
+ case _ => value.toString.toFloat
+ }
+ builder.setFloatVal(floatVal)
+
+ case "double" =>
+ val doubleVal = value match {
+ case d: java.lang.Double => d.doubleValue()
+ case f: java.lang.Float => f.doubleValue()
+ case _ => value.toString.toDouble
+ }
+ builder.setDoubleVal(doubleVal)
+
+ case "boolean" =>
+ val boolVal = value match {
+ case b: java.lang.Boolean => b.booleanValue()
+ case _ => value.toString.toBoolean
+ }
+ builder.setBoolVal(boolVal)
+
+ case "uuid" =>
+ // UUID as bytes (16 bytes) or string
+ val uuidBytes = value match {
+ case uuid: java.util.UUID =>
+ val bb = java.nio.ByteBuffer.wrap(new Array[Byte](16))
+ bb.putLong(uuid.getMostSignificantBits)
+ bb.putLong(uuid.getLeastSignificantBits)
+ bb.array()
+ case _ =>
+ // Parse UUID string and convert to bytes
+ val uuid = java.util.UUID.fromString(value.toString)
+ val bb = java.nio.ByteBuffer.wrap(new Array[Byte](16))
+ bb.putLong(uuid.getMostSignificantBits)
+ bb.putLong(uuid.getLeastSignificantBits)
+ bb.array()
+ }
+ builder.setUuidVal(com.google.protobuf.ByteString.copyFrom(uuidBytes))
+
+ case t if t.startsWith("fixed[") || t.startsWith("binary") =>
+ val bytes = value match {
+ case bytes: Array[Byte] => bytes
+ case _ => value.toString.getBytes("UTF-8")
+ }
+ if (t.startsWith("fixed")) {
+ builder.setFixedVal(com.google.protobuf.ByteString.copyFrom(bytes))
+ } else {
+ builder.setBinaryVal(com.google.protobuf.ByteString.copyFrom(bytes))
+ }
+
+ // Fallback: infer type from Java type ?
+ case _ =>
+ value match {
+ case s: String => builder.setStringVal(s)
+ case i: java.lang.Integer => builder.setIntVal(i.intValue())
+ case l: java.lang.Long => builder.setLongVal(l.longValue())
+ case d: java.lang.Double => builder.setDoubleVal(d.doubleValue())
+ case f: java.lang.Float => builder.setFloatVal(f.floatValue())
+ case b: java.lang.Boolean => builder.setBoolVal(b.booleanValue())
+ case other => builder.setStringVal(other.toString)
+ }
+ }
+ }
+
+ builder.build()
+ }
+
+ /**
+ * Legacy JSON serialization function - removed in favor of protobuf. Kept as reference for
+ * conversion logic.
*/
private def partitionValueToJson(fieldTypeStr: String, value: Any): JValue = {
Review Comment:
can we just remove this now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: [iceberg] Use protobuf instead of JSON to serialize Iceberg partition values [datafusion-comet]
Posted by "hsiang-c (via GitHub)" <gi...@apache.org>.
hsiang-c commented on code in PR #3247:
URL: https://github.com/apache/datafusion-comet/pull/3247#discussion_r2718918850
##########
spark/src/test/scala/org/apache/comet/IcebergReadFromS3Suite.scala:
##########
@@ -163,6 +163,41 @@ class IcebergReadFromS3Suite extends CometS3TestBase {
}
}
+ test("large scale partitioned table - 100 partitions with many files") {
+ assume(icebergAvailable, "Iceberg not available in classpath")
+
+ withSQLConf("spark.sql.files.maxRecordsPerFile" -> "50") {
+ spark.sql("""
+ CREATE TABLE s3_catalog.db.large_partitioned_test (
+ id INT,
+ data STRING,
+ partition_id INT
+ ) USING iceberg
+ PARTITIONED BY (partition_id)
+ """)
+
+ spark.sql("""
+ INSERT INTO s3_catalog.db.large_partitioned_test
+ SELECT
+ id,
+ CONCAT('data_', CAST(id AS STRING)) as data,
+ (id % 100) as partition_id
+ FROM range(500000)
+ """)
+
+ checkIcebergNativeScan(
+ "SELECT COUNT(DISTINCT id) FROM s3_catalog.db.large_partitioned_test")
+ checkIcebergNativeScan(
+ "SELECT * FROM s3_catalog.db.large_partitioned_test WHERE id < 10 ORDER BY id")
+ checkIcebergNativeScan(
+ "SELECT COUNT(*) FROM s3_catalog.db.large_partitioned_test WHERE partition_id = 0")
+ checkIcebergNativeScan(
+ "SELECT COUNT(*) FROM s3_catalog.db.large_partitioned_test WHERE partition_id IN (0, 50, 99)")
+
+ spark.sql("DROP TABLE s3_catalog.db.large_partitioned_test")
Review Comment:
(nit) You can try `DROP TABLE s3_catalog.db.large_partitioned_test PURGE` to remove files on disk.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: [iceberg] Use protobuf instead of JSON to serialize Iceberg partition values [datafusion-comet]
Posted by "parthchandra (via GitHub)" <gi...@apache.org>.
parthchandra commented on code in PR #3247:
URL: https://github.com/apache/datafusion-comet/pull/3247#discussion_r2719036610
##########
spark/src/main/scala/org/apache/comet/serde/operator/CometIcebergNativeScan.scala:
##########
@@ -71,14 +71,138 @@ object CometIcebergNativeScan extends CometOperatorSerde[CometBatchScanExec] wit
}
/**
- * Converts an Iceberg partition value to JSON format expected by iceberg-rust.
- *
- * iceberg-rust's Literal::try_from_json() expects specific formats for certain types:
- * - Timestamps: ISO string format "yyyy-MM-dd'T'HH:mm:ss.SSSSSS"
- * - Dates: ISO string format "YYYY-MM-DD"
- * - Decimals: String representation
- *
- * See: iceberg-rust/crates/iceberg/src/spec/values/literal.rs
+ * Converts an Iceberg partition value to protobuf format. Protobuf is less verbose than JSON.
+ * The following types are also serialized as integer values instead of as strings - Timestamps,
+ * Dates, Decimals, FieldIDs
+ */
+ private def partitionValueToProto(
+ fieldId: Int,
+ fieldTypeStr: String,
+ value: Any): OperatorOuterClass.PartitionValue = {
+ val builder = OperatorOuterClass.PartitionValue.newBuilder()
+ builder.setFieldId(fieldId)
+
+ if (value == null) {
+ builder.setIsNull(true)
+ } else {
+ builder.setIsNull(false)
+ fieldTypeStr match {
+ case t if t.startsWith("timestamp") =>
+ val micros = value match {
+ case l: java.lang.Long => l.longValue()
+ case i: java.lang.Integer => i.longValue()
+ case _ => value.toString.toLong
+ }
+ if (t.contains("tz")) {
+ builder.setTimestampTzVal(micros)
+ } else {
+ builder.setTimestampVal(micros)
+ }
+
+ case "date" =>
+ val days = value.asInstanceOf[java.lang.Integer].intValue()
+ builder.setDateVal(days)
+
+ case d if d.startsWith("decimal(") =>
+ // Serialize as unscaled BigInteger bytes
+ val bigDecimal = value match {
+ case bd: java.math.BigDecimal => bd
+ case _ => new java.math.BigDecimal(value.toString)
+ }
+ val unscaledBytes = bigDecimal.unscaledValue().toByteArray
+ builder.setDecimalVal(com.google.protobuf.ByteString.copyFrom(unscaledBytes))
+
+ case "string" =>
+ builder.setStringVal(value.toString)
+
+ case "int" =>
+ val intVal = value match {
+ case i: java.lang.Integer => i.intValue()
+ case l: java.lang.Long => l.intValue()
+ case _ => value.toString.toInt
+ }
+ builder.setIntVal(intVal)
+
+ case "long" =>
+ val longVal = value match {
+ case l: java.lang.Long => l.longValue()
+ case i: java.lang.Integer => i.longValue()
+ case _ => value.toString.toLong
+ }
+ builder.setLongVal(longVal)
+
+ case "float" =>
+ val floatVal = value match {
+ case f: java.lang.Float => f.floatValue()
+ case d: java.lang.Double => d.floatValue()
+ case _ => value.toString.toFloat
+ }
+ builder.setFloatVal(floatVal)
+
+ case "double" =>
+ val doubleVal = value match {
+ case d: java.lang.Double => d.doubleValue()
+ case f: java.lang.Float => f.doubleValue()
+ case _ => value.toString.toDouble
+ }
+ builder.setDoubleVal(doubleVal)
+
+ case "boolean" =>
+ val boolVal = value match {
+ case b: java.lang.Boolean => b.booleanValue()
+ case _ => value.toString.toBoolean
+ }
+ builder.setBoolVal(boolVal)
+
+ case "uuid" =>
+ // UUID as bytes (16 bytes) or string
+ val uuidBytes = value match {
+ case uuid: java.util.UUID =>
+ val bb = java.nio.ByteBuffer.wrap(new Array[Byte](16))
+ bb.putLong(uuid.getMostSignificantBits)
+ bb.putLong(uuid.getLeastSignificantBits)
+ bb.array()
+ case _ =>
+ // Parse UUID string and convert to bytes
+ val uuid = java.util.UUID.fromString(value.toString)
+ val bb = java.nio.ByteBuffer.wrap(new Array[Byte](16))
+ bb.putLong(uuid.getMostSignificantBits)
+ bb.putLong(uuid.getLeastSignificantBits)
+ bb.array()
+ }
+ builder.setUuidVal(com.google.protobuf.ByteString.copyFrom(uuidBytes))
+
+ case t if t.startsWith("fixed[") || t.startsWith("binary") =>
+ val bytes = value match {
+ case bytes: Array[Byte] => bytes
+ case _ => value.toString.getBytes("UTF-8")
+ }
+ if (t.startsWith("fixed")) {
+ builder.setFixedVal(com.google.protobuf.ByteString.copyFrom(bytes))
+ } else {
+ builder.setBinaryVal(com.google.protobuf.ByteString.copyFrom(bytes))
+ }
+
+ // Fallback: infer type from Java type ?
+ case _ =>
+ value match {
+ case s: String => builder.setStringVal(s)
+ case i: java.lang.Integer => builder.setIntVal(i.intValue())
+ case l: java.lang.Long => builder.setLongVal(l.longValue())
+ case d: java.lang.Double => builder.setDoubleVal(d.doubleValue())
+ case f: java.lang.Float => builder.setFloatVal(f.floatValue())
+ case b: java.lang.Boolean => builder.setBoolVal(b.booleanValue())
+ case other => builder.setStringVal(other.toString)
+ }
+ }
+ }
+
+ builder.build()
+ }
+
+ /**
+ * Legacy JSON serialization function - removed in favor of protobuf. Kept as reference for
+ * conversion logic.
*/
private def partitionValueToJson(fieldTypeStr: String, value: Any): JValue = {
Review Comment:
Found some unused code as a result of removing this. Thanks!
##########
native/core/src/execution/planner.rs:
##########
@@ -2589,6 +2650,105 @@ fn convert_spark_types_to_arrow_schema(
arrow_schema
}
+/// Converts a protobuf PartitionValue to an iceberg Literal.
+///
+/// This replaces JSON parsing with direct protobuf deserialization with a more compact
+/// representation (e.g., timestamps as integers vs strings).
Review Comment:
done
##########
native/proto/src/proto/operator.proto:
##########
@@ -110,6 +110,32 @@ message NativeScan {
bool encryption_enabled = 14;
}
+// Partition value for Iceberg partition data
+message PartitionValue {
+ int32 field_id = 1;
+ oneof value {
+ int32 int_val = 2;
+ int64 long_val = 3;
+ int64 date_val = 4; // days since epoch
+ int64 timestamp_val = 5; // microseconds since epoch
+ int64 timestamp_tz_val = 6; // microseconds with timezone
+ string string_val = 7;
+ double double_val = 8;
+ float float_val = 9;
+ bytes decimal_val = 10; // unscaled BigInteger bytes
+ bool bool_val = 11;
+ bytes uuid_val = 12;
+ bytes fixed_val = 13;
+ bytes binary_val = 14;
Review Comment:
These are Iceberg types though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org