Update extending-operators.md

[Adez017]

Which issue does this PR close?

• Closes Move code in user_defined_plan.rs to the extending-operators doc #15774

Rationale for this change

updated the extending-operators.md file

[Adez017]

hi @xudong963 , i want to ask that did we had to rewrite the part of code https://github.com/apache/datafusion/blob/main/datafusion/core/tests/user_defined/user_defined_plan.rs#L18-L24 afterwards or just had to migrate the whole code into extending-operators .
Also please review the changes that I made and provide feedback based on it

[xudong963]

i want to ask that did we had to rewrite the part of code https://github.com/apache/datafusion/blob/main/datafusion/core/tests/user_defined/user_defined_plan.rs#L18-L24 afterwards or just had to migrate the whole code into extending-operators .

I think after migrating them, we don't need to retain the code

[Adez017]

i want to ask that did we had to rewrite the part of code https://github.com/apache/datafusion/blob/main/datafusion/core/tests/user_defined/user_defined_plan.rs#L18-L24 afterwards or just had to migrate the whole code into extending-operators .

I think after migrating them, we don't need to retain the code

does that mean that I need to migrate all the code from user_defined_plan.rs to extending-operators with the code also ?

[xudong963]

i want to ask that did we had to rewrite the part of code https://github.com/apache/datafusion/blob/main/datafusion/core/tests/user_defined/user_defined_plan.rs#L18-L24 afterwards or just had to migrate the whole code into extending-operators .

I think after migrating them, we don't need to retain the code

does that mean that I need to migrate all the code from user_defined_plan.rs to extending-operators with the code also ?

Yes, except tests.

[Adez017]

i want to ask that did we had to rewrite the part of code https://github.com/apache/datafusion/blob/main/datafusion/core/tests/user_defined/user_defined_plan.rs#L18-L24 afterwards or just had to migrate the whole code into extending-operators .

I think after migrating them, we don't need to retain the code

does that mean that I need to migrate all the code from user_defined_plan.rs to extending-operators with the code also ?

Yes, except tests.

that means above it :

datafusion/datafusion/core/tests/user_defined/user_defined_plan.rs

Line 61 in 5d98d2c

use std::fmt::Debug;

[Adez017]

Hi @xudong963 , i think it is ready , give it a check

[xudong963]

You can refer to the doc: https://datafusion.apache.org/library-user-guide/custom-table-providers.html.

It should contain the real code https://github.com/apache/datafusion/blob/main/datafusion/core/tests/user_defined/user_defined_plan.rs#L458-L916 to describe the process of defining an extending operator, not only an SQL example.

[Adez017]

hey @xudong963 , check it out now .
also could you help in the failing check in workflow?

[xudong963]

You can rebase with main

[xudong963]

Would anyone happen to know how to preview the HTML format for the PR changes?

[Adez017]

You can rebase with main

doe this solve the issue ?

[xudong963]

You can rebase with main

doe this solve the issue ?

You can open the failed CI and see what's wrong:

error[E0599]: no method named `unwrap` found for struct `CoalescePartitionsExec` in the current scope
   --> datafusion/physical-optimizer/src/enforce_sorting/replace_with_order_preserving_variants.rs:210:14
    |
208 |           let coalesce = CoalescePartitionsExec::new(child)
    |  ________________________-
209 | |             .with_fetch(plan.fetch())
210 | |             .unwrap();
    | |             -^^^^^^ method not found in `CoalescePartitionsExec`
    | |_____________|
    |

warning: unused import: `ExecutionPlan`
  --> datafusion/physical-optimizer/src/enforce_sorting/replace_with_order_preserving_variants.rs:37:32
   |
37 | use datafusion_physical_plan::{ExecutionPlan, ExecutionPlanProperties};
   |                                ^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

The error is fixed in main, so rebasing your branch with main will fix the error

[Adez017]

You can rebase with main

doe this solve the issue ?

You can open the failed CI and see what's wrong:

error[E0599]: no method named `unwrap` found for struct `CoalescePartitionsExec` in the current scope
   --> datafusion/physical-optimizer/src/enforce_sorting/replace_with_order_preserving_variants.rs:210:14
    |
208 |           let coalesce = CoalescePartitionsExec::new(child)
    |  ________________________-
209 | |             .with_fetch(plan.fetch())
210 | |             .unwrap();
    | |             -^^^^^^ method not found in `CoalescePartitionsExec`
    | |_____________|
    |

warning: unused import: `ExecutionPlan`
  --> datafusion/physical-optimizer/src/enforce_sorting/replace_with_order_preserving_variants.rs:37:32
   |
37 | use datafusion_physical_plan::{ExecutionPlan, ExecutionPlanProperties};
   |                                ^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

The error is fixed in main, so rebasing your branch with main will fix the error

Thank for your help @xudong963 but I think it didn't work for the failing workflow

[xudong963]

You can rebase with main

doe this solve the issue ?

You can open the failed CI and see what's wrong:

error[E0599]: no method named `unwrap` found for struct `CoalescePartitionsExec` in the current scope
   --> datafusion/physical-optimizer/src/enforce_sorting/replace_with_order_preserving_variants.rs:210:14
    |
208 |           let coalesce = CoalescePartitionsExec::new(child)
    |  ________________________-
209 | |             .with_fetch(plan.fetch())
210 | |             .unwrap();
    | |             -^^^^^^ method not found in `CoalescePartitionsExec`
    | |_____________|
    |

warning: unused import: `ExecutionPlan`
  --> datafusion/physical-optimizer/src/enforce_sorting/replace_with_order_preserving_variants.rs:37:32
   |
37 | use datafusion_physical_plan::{ExecutionPlan, ExecutionPlanProperties};
   |                                ^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

The error is fixed in main, so rebasing your branch with main will fix the error

Thank for your help @xudong963 but I think it didn't work for the failing workflow

Sorry I am on vacation , you can try to fix by the error hints in ci

[Adez017]

i Think we need @alamb help now . could you help ?

[Adez017]

@alamb , please take a look

[Adez017]

could anyone please help here ?

[xudong963]

Could you please refer to the error lints in CI? Such as

error[E0433]: failed to resolve: use of undeclared type `Statistics`
  --> datafusion/core/src/lib.rs:1365:12
   |
98 |         Ok(Statistics::new_unknown(&self.schema()))
   |            ^^^^^^^^^^ use of undeclared type `Statistics`
   |
help: consider importing one of these items
   |
2  + use datafusion::physical_plan::Statistics;
   |
2  + use datafusion_common::Statistics;
   |
2  + use datafusion_physical_plan::Statistics;
   |
2  + use parquet::file::statistics::Statistics;

It's saying, you need to import Statistics for the example in the doc, such as

[alamb]

Thanks @Adez017 -- I took a brief look at this and I pushed some small changes.

I think the code will need some more work

[Adez017]

@alamb i think it perfect now and working . please take a look

[Adez017]

@alamb @xudong963 gentleman's tis need your attention now

[xudong963]

LGTM, thank you.

It's very close to the finish line. The last thing that we need is to preview the HTML. cc @alamb , could you please show me how to do the preview?

[xudong963]

[DataFusion Doc]?

[alamb]

Or maybe it should be a link to the source code - I was trying to avoid some link that would get out of date but maybe that in inevitable

[alamb]

LGTM, thank you.

It's very close to the finish line. The last thing that we need is to preview the HTML. cc @alamb , could you please show me how to do the preview?

I believe you can follow the steps here: https://github.com/apache/datafusion/tree/main/docs#build--preview

[alamb]

Thank you @Adez017 and @xudong963 -- I agree this is very close.

This current PR also adds a copy of adding a user defined plan. Maybe we can also remove the old copy?

[alamb]

Or maybe it should be a link to the source code - I was trying to avoid some link that would get out of date but maybe that in inevitable

[alamb]

This section now seems somewhat to duplicate the new sections

[Adez017]

i think we can rationalize it

[Adez017]

how about this ? we can write a separate guide or something for the µWheel , removing the content from this PR and make it as a separate . if you are agree @alamb , I would love to do the job if possible

[Adez017]

@alamb cc: @xudong963 check it now

[Adez017]

hey @alamb @xudong963 since your last visit I had made some changes . please take a look

[github-actions]

Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.

[Adez017]

hi @alamb please have a look on this

[alamb]

Thanks @Adez017

Thank you for your patience.

I think there are a few outstanding issues we should to resolve prior to merging this:

1. Remove the (now) redundant example, as specified in the original ticket #15774
2. Try and add a bit more detail to the examples.

Removing the existing example may be harder as it seems the test has morphed to also be testing user defined invariants.

I will give it a shot

[Adez017]

Thanks @Adez017

Thank you for your patience.

I think there are a few outstanding issues we should to resolve prior to merging this:

1. Remove the (now) redundant example, as specified in the original ticket Move code in user_defined_plan.rs to the extending-operators doc #15774
2. Try and add a bit more detail to the examples.

Removing the existing example may be harder as it seems the test has morphed to also be testing user defined invariants.

I will give it a shot

Hi @alamb, thank you for your time and efforts. i think there might be some updates that had happened since my last visit . i would love it if someone help me out with that