Skip to content

Conversation

@dillitz
Copy link
Contributor

@dillitz dillitz commented Sep 18, 2023

What changes were proposed in this pull request?

Adding a CommandPluginWithQueryPlanningTracker interface which extends the already existing CommandPlugin interface with a process() method that one can pass a QueryPlanningTracker to.

Why are the changes needed?

There is currently no way to track queries executed by a CommandPlugin.

Does this PR introduce any user-facing change?

Users now can also write trackable plugins by implementing the CommandPluginWithQueryPlanningTracker. Since this extends the CommandPlugin interface such plugins are also backward-compatible to older versions of spark only knowing the CommandPlugin interface.

How was this patch tested?

Test added.

Was this patch authored or co-authored using generative AI tooling?

No

@dillitz
Copy link
Contributor Author

dillitz commented Oct 10, 2023

#43311 solves this problem without the need to introduce a new interface.

@dillitz dillitz closed this Oct 10, 2023
hvanhovell pushed a commit that referenced this pull request Oct 11, 2023
### What changes were proposed in this pull request?
This PR adds an optional `ExecuteHolder` to `SparkConnectPlanner`.
This allows plugins to access the `ExecuteHolder` to for example create a `QueryPlanningTracker` without the need to create a new `CommandPlugin` interface like proposed in #42984.

### Why are the changes needed?
There is currently no way to track queries executed by a CommandPlugin. For this, the plugin needs access to the `ExecuteHolder`.

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Adjusted existing tests.

### Was this patch authored or co-authored using generative AI tooling?
No.

Closes #43311 from dillitz/plugin-new-approach.

Authored-by: Robert Dillitz <[email protected]>
Signed-off-by: Herman van Hovell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant