-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add copy hook utils module #29
Conversation
src/parquet_copy_hook/copy_utils.rs
Outdated
} | ||
|
||
pub(crate) fn validate_copy_from_options(pstmt: &PgBox<pg_sys::PlannedStmt>) { | ||
let allowed_options = ["format"]; |
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.
do we want to allow freeze
? (and propagate that into copy logic)
|
||
let mut found_parquet_format = false; | ||
let mut copy_options = unsafe { PgList::<DefElem>::from_pg(copy_stmt.options) }; | ||
for option in copy_options.iter_ptr() { |
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.
it wasn't quite clear to me why we're looking for the format here
this function seems to do more than its name suggests
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.
Made it copy_from_stmt_add_or_update_binary_format_option
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.
still a bit confusing to me, we probably want to build a new options list for internal use containing options like: format 'binary' (always), freeze (if present in the original list)
src/parquet_copy_hook/copy_utils.rs
Outdated
} | ||
} | ||
|
||
pub(crate) fn copy_relation_oid(pstmt: &PgBox<pg_sys::PlannedStmt>) -> Oid { |
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.
copy looks more like a verb here
maybe copy_statement_relation_oid, copy_statement_lock_mode
8109012
to
09698ac
Compare
3f87cc1
to
3627d3d
Compare
Addressed at |
No description provided.