You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@arrow.apache.org by "Light-City (via GitHub)" <gi...@apache.org> on 2023/08/15 06:57:55 UTC

[GitHub] [arrow] Light-City opened a new issue, #37170: [C++] Rewrite scheam in RecordBatch.

Light-City opened a new issue, #37170:
URL: https://github.com/apache/arrow/issues/37170

### Describe the enhancement requested

We have a scene. There is a plan in pg that looks like the following. For the Append node, there are two scans in parallel, and then there is a column of data, but the column names are different. If it is mapped to the arrow schema It is a different field. For the append node, we will get two batches. The first batch comes from the first scan, and the second batch comes from the second scan, but because the two columns are constructed based on the scan The schema is different, so the final schema of the two batches is different. When we construct the slot returned by the Append node, we use the schema of the first batch. When we put the data of the second batch into it, the verification fails due to inconsistent shcema.

**Therefore, the problem is simplified to: For a node, If there are n child nodes, the schema of the following child nodes must be consistent. If not, the schema of n-1 child nodes must be the same as the first schema, so there is logic to rewrite the schema of the batch data.**

```
-> Vec Append
-> Vec Seq Scan on public. tenk1
Output: tenk1.unique1
-> Vec Seq Scan on public.tenk1 tenk1_1
Output: tenk1_1.fivethous
```

However, when reading the batch code, there is only the read-only interface `schema()`, so here we submit a pr to add and rewrite the schema interface, and only modify the columns with the same type. If they are not the same, an invalid modification will be returned.

### Component(s)

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

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

[GitHub] [arrow] Light-City commented on issue #37170: [C++] Rewrite schema in RecordBatch.

Posted by "Light-City (via GitHub)" <gi...@apache.org>.

Light-City commented on issue #37170:
URL: https://github.com/apache/arrow/issues/37170#issuecomment-1678535991

   > I guess `Project` will not always modify the data, though it will call an expression.
   > 
   > Another point is that `RecordBatch` seems tent to be immutable. Would the interface below be ok?
   > 
   > ```c++
   > Result<std::shared_ptr<RecordBatch>> SetSchema(...);
   > ```
   
   I agree, if RecordBatch is read-only from a design point of view, then it really needs to be designed as above,


-- 
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@arrow.apache.org

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

[GitHub] [arrow] Light-City commented on issue #37170: [C++] Rewrite schema in RecordBatch.

Posted by "Light-City (via GitHub)" <gi...@apache.org>.

Light-City commented on issue #37170:
URL: https://github.com/apache/arrow/issues/37170#issuecomment-1678519078

   > What about using a `Project` node? Seems that Acero can unify the schema with the help of Union or Project?
   
   I personally feel that it is too heavy, and I have to start an arrow plan again. From the perspective of batch, if I only want to modify the schema name, I still need to rewrite the schema interface


-- 
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@arrow.apache.org

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

[GitHub] [arrow] lidavidm closed issue #37170: [C++] Rewrite schema in RecordBatch.

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.

lidavidm closed issue #37170: [C++] Rewrite schema in RecordBatch.
URL: https://github.com/apache/arrow/issues/37170


-- 
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@arrow.apache.org

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

[GitHub] [arrow] mapleFU commented on issue #37170: [C++] Rewrite schema in RecordBatch.

Posted by "mapleFU (via GitHub)" <gi...@apache.org>.

mapleFU commented on issue #37170:
URL: https://github.com/apache/arrow/issues/37170#issuecomment-1678531040

   I guess `Project` will not always modify the data, though it will call an expression.
   
   Another point is that `RecordBatch` seems tent to be immutable. Would the interface below be ok?
   
   ```c++
   Result<std::shared_ptr<RecordBatch>> SetSchema(...);
   ```


-- 
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@arrow.apache.org

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

[GitHub] [arrow] Light-City commented on issue #37170: [C++] Rewrite schema in RecordBatch.

Posted by "Light-City (via GitHub)" <gi...@apache.org>.

Light-City commented on issue #37170:
URL: https://github.com/apache/arrow/issues/37170#issuecomment-1685503912

   > ```c++
   > SetSchema
   > ```
   
   schema_ is readonly, so we don't use SetSchema function to get batch schema.
   
   ```
    protected:
     RecordBatch(const std::shared_ptr<Schema>& schema, int64_t num_rows);
   
     std::shared_ptr<Schema> schema_;
     int64_t num_rows_;
   ```


-- 
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@arrow.apache.org

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

[GitHub] [arrow] Light-City commented on issue #37170: [C++] Rewrite schema in RecordBatch.

Posted by "Light-City (via GitHub)" <gi...@apache.org>.

Light-City commented on issue #37170:
URL: https://github.com/apache/arrow/issues/37170#issuecomment-1685504318

   maybe RewriteSchema in recordbatch is a good idea
   


-- 
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@arrow.apache.org

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

[GitHub] [arrow] mapleFU commented on issue #37170: [C++] Rewrite schema in RecordBatch.

Posted by "mapleFU (via GitHub)" <gi...@apache.org>.

mapleFU commented on issue #37170:
URL: https://github.com/apache/arrow/issues/37170#issuecomment-1678515864

   What about using a `Project` node? Seems that Acero can unify the schema with the help of Union or Project?


-- 
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@arrow.apache.org

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