Skip to content

[Data] Add descriptive error if user tries to download from invalid column#56852

Merged
bveeramani merged 1 commit intomasterfrom
add-error-download
Sep 24, 2025
Merged

[Data] Add descriptive error if user tries to download from invalid column#56852
bveeramani merged 1 commit intomasterfrom
add-error-download

Conversation

@bveeramani
Copy link
Member

@bveeramani bveeramani commented Sep 23, 2025

Why are these changes needed?

Before

KeyError: 'Field "eggs" does not exist in schema'

After

ValueError: Ray Data tried to download URIs in a column named 'eggs', but a column with that name doesn't exist. Is the specified download column correct?

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@bveeramani
Initiual commiT
9d1f602 
Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
@bveeramani bveeramani requested a review from a team as a code owner September 23, 2025 21:42
omatthew98
omatthew98 approved these changes Sep 23, 2025
View reviewed changes
Copy link
Contributor

@omatthew98 omatthew98 left a comment

Choose a reason for hiding this comment

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

💯

gemini-code-assist[bot]
gemini-code-assist bot reviewed Sep 23, 2025
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request improves the user experience by providing a more descriptive error message when download() is used with a non-existent column. The implementation is on the right track, but the fix is incomplete as it doesn't cover all code paths. The new check is only added to PartitionActor, which is bypassed in certain scenarios like chained download operations. I've left a detailed comment on how to make the fix comprehensive by adding a similar check in another location and suggested a small refactoring to avoid code duplication.

cursor[bot]
cursor bot reviewed Sep 23, 2025
View reviewed changes
python/ray/data/_internal/planner/plan_download_op.py
"Ray Data tried to download URIs from a column named "
f"{self._uri_column_name!r}, but a column with that name doesn't "
"exist. Is the specified download column correct?"
)
Copy link

@cursor cursor bot Sep 23, 2025

Choose a reason for hiding this comment

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

Bug: Column Check Fails on Non-PyArrow Tables

The column existence check accesses block.column_names before the block is guaranteed to be a PyArrow table. If the input isn't a PyArrow table (e.g., a pandas DataFrame), this can raise an AttributeError or lead to incorrect results.

Fix in Cursor Fix in Web

@bveeramani bveeramani enabled auto-merge (squash) September 23, 2025 23:39
@github-actions github-actions bot added the go add ONLY when ready to merge, run all tests label Sep 23, 2025
@bveeramani bveeramani merged commit c4b0335 into master Sep 24, 2025
7 checks passed
@bveeramani bveeramani deleted the add-error-download branch September 24, 2025 00:49
marcostephan pushed a commit to marcostephan/ray that referenced this pull request Sep 24, 2025
@bveeramani @marcostephan
[Data] Add descriptive error if user tries to download from invalid c…
c9451e2 
…olumn (ray-project#56852)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?

<!-- Please give a short summary of the change and the problem this
solves. -->

**Before**

> KeyError: 'Field "eggs" does not exist in schema'

**After**

> ValueError: Ray Data tried to download URIs in a column named 'eggs',
but a column with that name doesn't exist. Is the specified download
column correct?

## Related issue number

<!-- For example: "Closes ray-project#1234" -->

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: Marco Stephan <marco@magic.dev>
elliot-barn pushed a commit that referenced this pull request Sep 27, 2025
@bveeramani @elliot-barn
[Data] Add descriptive error if user tries to download from invalid c…
7a46588 
…olumn (#56852)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?

<!-- Please give a short summary of the change and the problem this
solves. -->

**Before**

> KeyError: 'Field "eggs" does not exist in schema'

**After**

> ValueError: Ray Data tried to download URIs in a column named 'eggs',
but a column with that name doesn't exist. Is the specified download
column correct?

## Related issue number

<!-- For example: "Closes #1234" -->

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
dstrodtman pushed a commit that referenced this pull request Oct 6, 2025
@bveeramani @dstrodtman
[Data] Add descriptive error if user tries to download from invalid c…
894c6a2 
…olumn (#56852)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?

<!-- Please give a short summary of the change and the problem this
solves. -->

**Before**

> KeyError: 'Field "eggs" does not exist in schema'

**After**

> ValueError: Ray Data tried to download URIs in a column named 'eggs',
but a column with that name doesn't exist. Is the specified download
column correct?

## Related issue number

<!-- For example: "Closes #1234" -->

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: Douglas Strodtman <douglas@anyscale.com>
justinyeh1995 pushed a commit to justinyeh1995/ray that referenced this pull request Oct 20, 2025
@bveeramani @justinyeh1995
[Data] Add descriptive error if user tries to download from invalid c…
188d86e 
…olumn (ray-project#56852)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?

<!-- Please give a short summary of the change and the problem this
solves. -->

**Before**

> KeyError: 'Field "eggs" does not exist in schema'

**After**

> ValueError: Ray Data tried to download URIs in a column named 'eggs',
but a column with that name doesn't exist. Is the specified download
column correct?

## Related issue number

<!-- For example: "Closes ray-project#1234" -->

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
landscapepainter pushed a commit to landscapepainter/ray that referenced this pull request Nov 17, 2025
@bveeramani @landscapepainter
[Data] Add descriptive error if user tries to download from invalid c…
3f17882 
…olumn (ray-project#56852)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?

<!-- Please give a short summary of the change and the problem this
solves. -->

**Before**

> KeyError: 'Field "eggs" does not exist in schema'

**After**

> ValueError: Ray Data tried to download URIs in a column named 'eggs',
but a column with that name doesn't exist. Is the specified download
column correct?

## Related issue number

<!-- For example: "Closes ray-project#1234" -->

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Future-Outlier pushed a commit to Future-Outlier/ray that referenced this pull request Dec 7, 2025
@bveeramani @Future-Outlier
[Data] Add descriptive error if user tries to download from invalid c…
a6e0f94 
…olumn (ray-project#56852)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?

<!-- Please give a short summary of the change and the problem this
solves. -->

**Before**

> KeyError: 'Field "eggs" does not exist in schema'

**After**

> ValueError: Ray Data tried to download URIs in a column named 'eggs',
but a column with that name doesn't exist. Is the specified download
column correct?

## Related issue number

<!-- For example: "Closes ray-project#1234" -->

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: Future-Outlier <eric901201@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments

@omatthew98 omatthew98 omatthew98 approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@omatthew98 omatthew98

Labels

go add ONLY when ready to merge, run all tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bveeramani @omatthew98