Skip to content

[native] Add TextReader and TextWriter#25626

Open
YiChengLee03 wants to merge 1 commit intoprestodb:masterfrom
YiChengLee03:master
Open

[native] Add TextReader and TextWriter#25626
YiChengLee03 wants to merge 1 commit intoprestodb:masterfrom
YiChengLee03:master

Conversation

@YiChengLee03
Copy link
Copy Markdown

@YiChengLee03 YiChengLee03 commented Jul 25, 2025

Description

Register Reader and Writer

Motivation and Context

Add support for Textfile reader and writer

Impact

Test Plan

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== NO RELEASE NOTE ==

@YiChengLee03 YiChengLee03 marked this pull request as ready for review July 25, 2025 22:31
@YiChengLee03 YiChengLee03 requested a review from a team as a code owner July 25, 2025 22:31
@xin-zhang2 xin-zhang2 mentioned this pull request Jul 29, 2025
6 tasks
@aditi-pandit
Copy link
Copy Markdown
Contributor

@YiChengLee03 : Thanks for these code changes.

Please can you add some e2e tests for this PR.

If Text is a valid Hive storage format, then it would be good to add this as a storage format in https://github.com/prestodb/presto/blob/master/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/PrestoNativeQueryRunnerUtils.java#L92, so that we can easily flip Native tests to use text format (just like we can switch between DWRF and Parquet).

@amitkdutta @xin-zhang2

zacw7
zacw7 previously approved these changes Jul 29, 2025
@YiChengLee03
Copy link
Copy Markdown
Author

@YiChengLee03 : Thanks for these code changes.

Please can you add some e2e tests for this PR.

If Text is a valid Hive storage format, then it would be good to add this as a storage format in https://github.com/prestodb/presto/blob/master/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/PrestoNativeQueryRunnerUtils.java#L92, so that we can easily flip Native tests to use text format (just like we can switch between DWRF and Parquet).

@amitkdutta @xin-zhang2

Sure! Thanks for catching that!

@YiChengLee03 YiChengLee03 marked this pull request as draft July 29, 2025 21:37
@YiChengLee03 YiChengLee03 force-pushed the master branch 5 times, most recently from b7963b7 to b168d30 Compare July 30, 2025 16:59
@YiChengLee03 YiChengLee03 requested a review from zacw7 July 30, 2025 17:01
@YiChengLee03 YiChengLee03 marked this pull request as ready for review July 30, 2025 17:01
@YiChengLee03 YiChengLee03 marked this pull request as draft July 30, 2025 17:54
@YiChengLee03
Copy link
Copy Markdown
Author

Build failure tracked here and fixed here

@YiChengLee03 YiChengLee03 marked this pull request as ready for review August 1, 2025 17:41
@libianoss
Copy link
Copy Markdown

@YiChengLee03 @zacw7 can we get this PR approved and merge?Thanks.

@xin-zhang2
Copy link
Copy Markdown
Contributor

xin-zhang2 commented Sep 2, 2025

@YiChengLee03 @zacw7 Are you still working on this PR? If not, I can help with it.

@zacw7
Copy link
Copy Markdown
Member

zacw7 commented Sep 2, 2025

@YiChengLee03 @zacw7 Are you still working on this PR? If not, I can help with it.

Please feel free to take it over. I can also resubmit this change.

@xin-zhang2
Copy link
Copy Markdown
Contributor

@zacw7 Thanks! TextWriter has been added in #25673, and I've submmitted a new PR #25995 for the TextReader.

zacw7 pushed a commit that referenced this pull request Dec 5, 2025
## Description
<!---Describe your changes in detail-->
Add TextReader registration.
It's a continuation of #25626.


## Motivation and Context
<!---Why is this change required? What problem does it solve?-->
<!---If it fixes an open issue, please link to the issue here.-->

## Impact
<!---Describe any public API or user-facing feature change or any
performance impact-->

## Test Plan
<!---Please fill in how you tested your change-->

## Contributor checklist

- [ ] Please make sure your submission complies with our [contributing
guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md),
in particular [code
style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style)
and [commit
standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards).
- [ ] PR description addresses the issue accurately and concisely. If
the change is non-trivial, a GitHub Issue is referenced.
- [ ] Documented new properties (with its default value), SQL syntax,
functions, or other functionality.
- [ ] If release notes are required, they follow the [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines).
- [ ] Adequate tests were added if applicable.
- [ ] CI passed.

## Release Notes
Please follow [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines)
and fill in the release notes below.

```
== RELEASE NOTES ==

Prestissimo (Native Execution) Changes
*  Add TextReader support for tables in TEXTFILE format. 

```
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.

5 participants