Skip to content

Raise a clear error for unsupported ROAPI partition column types - #76

Merged
saarthak2002 merged 1 commit into
neuralinkcorp:mainfrom
SID-6921:fix/roapi-unsupported-partition-type
Oct 5, 2026
Merged

saarthak2002 merged 1 commit into
neuralinkcorp:mainfrom
SID-6921:fix/roapi-unsupported-partition-type

Conversation

@SID-6921

@SID-6921 SID-6921 commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Summary

`py_type_to_roapi` (src/datarepo/export/roapi.py) maps a partition column's Python value type to a ROAPI data type via a bare dict subscript:

```python
return {int: "Int64", str: "Utf8", bool: "Boolean", float: "Float64"}[py_type]
```

Any `docs_filters` value whose type isn't one of those four (e.g. a `datetime.date` partition column, which is a very ordinary thing to partition by) blows up `export_to_roapi_table` with an opaque `KeyError: <class 'datetime.date'>`, instead of a message that says what's actually unsupported. The rest of this codebase's filter/operator conversion code (`_filter_to_expr`, `filter_to_sql_expr`, `ClickHouseTable._build_query`) consistently raises a `ValueError` naming the bad input — this was the one spot still doing a raw subscript.

Changes

  • `py_type_to_roapi` now raises `ValueError(f"Unsupported partition column type {py_type!r} for ROAPI export. Supported types: [...]")` for anything outside the four supported types.
  • No change in behavior for the four supported types.

Test plan

  • New test fails on `main` with `KeyError: <class 'datetime.date'>` (verified by reverting the source change locally and re-running)
  • New tests pass with the fix, including a parametrized check that all four previously-supported types are unaffected
  • Full `test/export/` suite passes (18 passed)
  • `black --check` clean on both changed files

py_type_to_roapi did a bare dict subscript, so any partition value type
outside {int, str, bool, float} (e.g. datetime.date) raised an opaque
KeyError: <class 'datetime.date'> instead of saying what was wrong.
Matches the ValueError-on-bad-input convention used elsewhere in the
filter/operator conversion code.
Copilot AI balanced review requested due to automatic review settings October 2, 2026 23:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@neuralink-code-review-bot neuralink-code-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No material issues. The change replaces an opaque KeyError with a ValueError that names the unsupported type, matching the rest of the filter/export conversion code, and the four previously supported mappings are unchanged. Coverage includes both the helper and the export_to_roapi_table path for a date partition value.

Posted by the code review bot. This is an automated review, not a maintainer approval.

@saarthak2002 saarthak2002 self-assigned this Oct 5, 2026
@saarthak2002
saarthak2002 merged commit 6a6c5de into neuralinkcorp:main Oct 5, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants