Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 2.3k
DRAFT implement trait in proto de/serialization to allow finer control#18813
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
3c88d0c3804e892733289c70abd74b0eef5ce639c961b8a35e9992ee4635720File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -63,11 +63,11 @@ impl TryFrom<AccumulatorArgs<'_>> for FFI_AccumulatorArgs { | ||
| WrappedSchema(FFI_ArrowSchema::try_from(args.return_field.as_ref())?); | ||
| let schema = WrappedSchema(FFI_ArrowSchema::try_from(args.schema)?); | ||
| let codec = DefaultPhysicalExtensionCodec {}; | ||
| let mut codec = DefaultPhysicalExtensionCodec {}; | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a plan @timsaucer to make this configurable so we can use our own codec? It’s likely covered in other PRs, is it? MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that is my ultimate goal. I don't want to merge any PR until I've got at least a hacked together demo of it with FFI and used in df-python. | ||
| let ordering_req = | ||
| serialize_physical_sort_exprs(args.order_bys.to_owned(), &codec)?; | ||
| serialize_physical_sort_exprs(args.order_bys.to_owned(), &mut codec)?; | ||
| let expr = serialize_physical_exprs(args.exprs, &codec)?; | ||
| let expr = serialize_physical_exprs(args.exprs, &mut codec)?; | ||
| let physical_expr_def = PhysicalAggregateExprNode { | ||
| expr, | ||
| @@ -121,17 +121,12 @@ impl TryFrom<FFI_AccumulatorArgs> for ForeignAccumulatorArgs { | ||
| let schema = Schema::try_from(&value.schema.0)?; | ||
| let default_ctx = SessionContext::new(); | ||
| let task_ctx = default_ctx.task_ctx(); | ||
| let codex = DefaultPhysicalExtensionCodec {}; | ||
| let mut task_ctx = default_ctx.task_ctx(); | ||
| let order_bys = parse_physical_sort_exprs( | ||
| &proto_def.ordering_req, | ||
| &task_ctx, | ||
| &schema, | ||
| &codex, | ||
| )?; | ||
| let order_bys = | ||
| parse_physical_sort_exprs(&proto_def.ordering_req, &mut task_ctx, &schema)?; | ||
| let exprs = parse_physical_exprs(&proto_def.expr, &task_ctx, &schema, &codex)?; | ||
| let exprs = parse_physical_exprs(&proto_def.expr, &mut task_ctx, &schema)?; | ||
| let expr_fields = exprs | ||
| .iter() | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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.
I find
parseras a name may be a bit confusing