Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-10034: [Rust] Fix Rust build on master #8213

Closed
wants to merge 2 commits into from

Conversation

andygrove
Copy link
Member

@andygrove andygrove commented Sep 17, 2020

Reverts ARROW-9977 and fixes one other conflict.

I was seeing compilation errors like this:

error[E0599]: no method named `value` found for reference `&array::array::StringArray` in the current scope
  --> arrow/src/compute/kernels/aggregate.rs:39:35
   |
39 |                 let item = $array.value(i);
   |                                   ^^^^^ method not found in `&array::array::StringArray`

@andygrove
Copy link
Member Author

@jorgecarleitao fyi

@andygrove
Copy link
Member Author

build error seems unrelated so I am going to go ahead and merge this

@andygrove andygrove closed this in 84b1512 Sep 18, 2020
@jorgecarleitao
Copy link
Member

Thanks a lot for the quick response on solving this.

One was related to a new trait in Arrow, StringArrayOps, that has .value(i) -> &str, and is needed when using value in strings, in the commit for take.

The other one is related to the Ops in logical plans.

Essentially, this is all related to multiple merges, with backward incompatible changes on them.

@andygrove , is there anything we can do to help mitigating this during merge of multiple merges?

One idea is to have a WARNING in the PR text that the merge is backward incompatible, so that the committer knows of potential conflicts with other merges.

Another idea is to first perform the merges locally, one by one, and then push a branch with those changes to test them in integration, and then merge them at once.

@andygrove
Copy link
Member Author

andygrove commented Sep 18, 2020 via email

emkornfield pushed a commit to emkornfield/arrow that referenced this pull request Oct 16, 2020
Reverts ARROW-9977 and fixes one other conflict.

I was seeing compilation errors like this:

```
error[E0599]: no method named `value` found for reference `&array::array::StringArray` in the current scope
  --> arrow/src/compute/kernels/aggregate.rs:39:35
   |
39 |                 let item = $array.value(i);
   |                                   ^^^^^ method not found in `&array::array::StringArray`
```

Closes apache#8213 from andygrove/fix-rust-build-sep17

Authored-by: Andy Grove <andygrove@nvidia.com>
Signed-off-by: Andy Grove <andygrove@nvidia.com>
GeorgeAp pushed a commit to sirensolutions/arrow that referenced this pull request Jun 7, 2021
Reverts ARROW-9977 and fixes one other conflict.

I was seeing compilation errors like this:

```
error[E0599]: no method named `value` found for reference `&array::array::StringArray` in the current scope
  --> arrow/src/compute/kernels/aggregate.rs:39:35
   |
39 |                 let item = $array.value(i);
   |                                   ^^^^^ method not found in `&array::array::StringArray`
```

Closes apache#8213 from andygrove/fix-rust-build-sep17

Authored-by: Andy Grove <andygrove@nvidia.com>
Signed-off-by: Andy Grove <andygrove@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants