You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "liurenjie1024 (via GitHub)" <gi...@apache.org> on 2023/08/17 09:00:28 UTC

[GitHub] [iceberg-rust] liurenjie1024 opened a new issue, #32: Add transform function implementation.

liurenjie1024 opened a new issue, #32:
URL: https://github.com/apache/iceberg-rust/issues/32

   (no comment)


-- 
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: issues-unsubscribe@iceberg.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg-rust] liurenjie1024 commented on issue #32: Add transform function implementation.

Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on issue #32:
URL: https://github.com/apache/iceberg-rust/issues/32#issuecomment-1698421451

   > Do you think it's possible to use the capabilities of the query engine to repartition the data? Maybe there are some optimizations that can be performed if this is expressed as an Repartition Operator.
   
   Yeah, it's possible. However, usually we don't bind compute task to one partition of storage format, since the parallelism of of compute tasks is determined by many factors, not just table partition. I think the hierarchy of iceberg's `TaskWriter` explains things well enough:
   
   <img width="461" alt="image" src="https://github.com/apache/iceberg-rust/assets/2771941/f8fb3128-d22f-4e0b-80c0-9592f075e748">
   
   `PartitionedWriter` is used when data is partitioned and sorted according to partition value.
   
   `PartitionedFanoutWriter` is used when data not partitioned by shuffle, and need to be precessed in writer.
   


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg-rust] ZENOTME commented on issue #32: Add transform function implementation.

Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on issue #32:
URL: https://github.com/apache/iceberg-rust/issues/32#issuecomment-1697711746

    > Do you think it's possible to use the capabilities of the query engine to repartition the data? May be there are some optimizations that can be performed if this is expressed as an Repartition Operator.
   
   Good point! In icelake, the repartition work is doing by HashMap and arrow provide [the effiect way](https://docs.rs/arrow-row/46.0.0/arrow_row/index.html) to hash the partition value. And as you says this work maybe can express as [Repartition Operator](https://arrow.apache.org/datafusion/user-guide/dataframe.html#:~:text=Repartition%20a%20DataFrame%20based%20on%20a%20logical%20partitioning%20scheme.). We can take the investigate.
   
   
   


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [I] Add transform function implementation. [iceberg-rust]

Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 closed issue #32: Add transform function implementation.
URL: https://github.com/apache/iceberg-rust/issues/32


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg-rust] liurenjie1024 commented on issue #32: Add transform function implementation.

Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on issue #32:
URL: https://github.com/apache/iceberg-rust/issues/32#issuecomment-1690994903

   Blocked by #38


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg-rust] liurenjie1024 commented on issue #32: Add transform function implementation.

Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on issue #32:
URL: https://github.com/apache/iceberg-rust/issues/32#issuecomment-1697308710

   > Regarding the representation of partition values which relates to the use of transform. I'm wondering whether the result of a transform should be a scalar value. There should only be one partition value for each partition which is stored in the datafile. And the partition values are not written to disk.
   
   Yeah, I can understand that it's a little difficult to understand why we should implement on arrow. I will use a simplified partition writer to explain this:
   ```rust
   struct PartitionWriter {
     transform_func: Box<dyn TransformFunction>,
     parquet_writers: HashMap<PartitionValue, ParquetWriter>
   }
   
   impl PartitionWriter {
     fn append(record_batch: RecordBatch)  {
        let partition_values = transform_func.apply(record_batch.get(0)).map(|arrow_row| to_iceberg_partition_value(arrow_row))?; // We assume partition function should apply on the first column
       
        for i in 0..record_batch.row_count() {
         parquet_writers.get(partition_values.get(i)).append(record_batch.rows(i));
   }
        
     }
   }
   ```
   
   With this approach, we can compute partiton values in a vectorized approach. For more detailed implementation, please refer to  https://github.com/icelake-io/icelake/blob/aeba46c62932b482340d34550289a1b3a9f30486/icelake/src/io/task_writer.rs#L175


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg-rust] JanKaul commented on issue #32: Add transform function implementation.

Posted by "JanKaul (via GitHub)" <gi...@apache.org>.
JanKaul commented on issue #32:
URL: https://github.com/apache/iceberg-rust/issues/32#issuecomment-1697189676

   Regarding the representation of partition values which relates to the use of transform. I'm wondering whether the result of a transform should be a scalar value. There should only be one partition value for each partition which is stored in the datafile. And the partition values are not written to 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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg-rust] ZENOTME commented on issue #32: Add transform function implementation.

Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on issue #32:
URL: https://github.com/apache/iceberg-rust/issues/32#issuecomment-1692724564

   > cc @ZENOTME Coule you help to port our implementation here later?
   
   Sure. I'm implementing the transform function for temporal type in icelake now. I will port them gradually later. 


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg-rust] JanKaul commented on issue #32: Add transform function implementation.

Posted by "JanKaul (via GitHub)" <gi...@apache.org>.
JanKaul commented on issue #32:
URL: https://github.com/apache/iceberg-rust/issues/32#issuecomment-1697427261

   Ah okay, thanks for the explanation. So the reason is to "multiplex" the rows into the correct parquet writer according to its partition value.
   
   Do you think it's possible to use the capabilities of the query engine to repartition the data? Maybe there are some optimizations that can be performed if this is expressed as an Repartition Operator.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg-rust] liurenjie1024 commented on issue #32: Add transform function implementation.

Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on issue #32:
URL: https://github.com/apache/iceberg-rust/issues/32#issuecomment-1692708504

   From our experience in developing https://github.com/icelake-io/icelake, we should implement it on arrow array when introducing partition writer.
   
   cc @ZENOTME  Coule you help to port our implementation here later?


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg-rust] JanKaul commented on issue #32: Add transform function implementation.

Posted by "JanKaul (via GitHub)" <gi...@apache.org>.
JanKaul commented on issue #32:
URL: https://github.com/apache/iceberg-rust/issues/32#issuecomment-1698481306

   That makes sense. Thanks again for the explanation.
   
   Generally it makes sense to have a PartitionWriter that doesn't depend on a query engine. Then this could be used in other libraries.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [I] Add transform function implementation. [iceberg-rust]

Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on issue #32:
URL: https://github.com/apache/iceberg-rust/issues/32#issuecomment-1855092304

   Close since already finished.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org