-
Notifications
You must be signed in to change notification settings - Fork 425
refactor: convert to postgres values directly from arrow #7131
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the PostgreSQL type encoding logic to convert data directly from Arrow arrays instead of going through intermediate Value representations. This follows a similar optimization made in PR #7096.
Key Changes
- Replaced value-based encoding with direct Arrow array conversion for better performance
- Introduced
RecordBatchRowIteratorto iterate over record batches and encode rows directly from Arrow arrays - Eliminated intermediate
Valueconversions by reading directly from typed Arrow arrays
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| src/servers/src/postgres/types.rs | Refactored encoding functions to work directly with Arrow arrays; added RecordBatchRowIterator for row-by-row encoding; updated tests to use Arrow schema and vectors |
| src/servers/src/postgres/handler.rs | Simplified stream processing to use new RecordBatchRowIterator instead of manual row encoding |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
70323c6 to
df2efc0
Compare
Signed-off-by: luofucong <[email protected]>
df2efc0 to
24fd6e5
Compare
|
LGTM. And I think next step we can switch to my datafusion-postgres ecosystem library arrow-pg |
Signed-off-by: luofucong <[email protected]>
|
@killme2008 PTAL |
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
What's changed and what's your intention?
following #7096
for postgres
PR Checklist
Please convert it to a draft if some of the following conditions are not met.