Skip to content

feat(irx): add utf8 and large_utf8 string columns to record batch#93

Open
yogendra-17 wants to merge 2 commits into
arxlang:mainfrom
yogendra-17:feat/arrow-string-temporal
Open

feat(irx): add utf8 and large_utf8 string columns to record batch#93
yogendra-17 wants to merge 2 commits into
arxlang:mainfrom
yogendra-17:feat/arrow-string-temporal

Conversation

@yogendra-17

Copy link
Copy Markdown
Contributor

Pull Request description

This PR adds string columns so IRx record batches can hold text, still fully Arrow-compatible.

What changed

  • Added two text column types: UTF8 (normal strings) and LARGE_UTF8 (64-bit offsets, for very large text).
  • append_string(...) to write text and get_string(...) to read it back handles any language and emoji (multi-byte UTF-8), empty strings, and null/missing values.
  • Wired through all three layers that must agree: the native C++ library (irx_record_batch.cpp/.h), the Python ctypes wrapper (irx/record_batch.py), and the RuntimeFeature symbol table(builder/runtime/record_batch.py) so compiled IRx programs get the symbols too.

How to test these changes

  • ...

Pull Request checklists

This PR is a:

  • bug-fix
  • new feature
  • maintenance

About this PR:

  • it includes tests.
  • the tests are executed on CI.
  • the tests generate log file(s) (path).
  • pre-commit hooks were executed locally.
  • this PR requires a project documentation update.

Author's checklist:

  • I have reviewed the changes and it contains no misspelling.
  • The code is well commented, especially in the parts that contain more
    complexity.
  • New and old tests passed locally.

Additional information

This is the string half of (string + temporal types), built on the Arrow C++ bridge from #88. Temporal types (date/timestamp) follow in a separate PR to keep this one small and reviewable.

Reviewer's checklist

Copy and paste this template for your review's note:

## Reviewer's Checklist

- [ ] I managed to reproduce the problem locally from the `main` branch
- [ ] I managed to test the new changes locally
- [ ] I confirm that the issues mentioned were fixed/resolved .

return IRX_OK;
}

int irx_rb_batch_get_utf8(const IrxRbBatch *b, int col, int64_t row,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

on a null slot this returns an empty view rather than an error. That's consistent with the numeric getters, so no change needed here, but please add a line to the get_string docstring in record_batch.py telling callers to check is_null first.

builder.release()
schema.release()

def test_wrong_column_type_raises(self):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This tests the append-side type mismatch, but the get-side branch (irx_rb_batch_get_utf8 on a non-string column) is untested, add a case that builds an INT32 batch and asserts get_string raises.

@yuvimittal

yuvimittal commented Jul 14, 2026

Copy link
Copy Markdown
Member

@yogendra-17 , can you please rebase your branch ? The core design is correct, thanks for working on that. I have added some small comments, but we can push them later

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.

2 participants