Skip to content

Move datafusion-cli main_inner impl into the datafusion-cli lib.#23387

Open
ratmice wants to merge 4 commits into
apache:mainfrom
ratmice:cli_session
Open

Move datafusion-cli main_inner impl into the datafusion-cli lib.#23387
ratmice wants to merge 4 commits into
apache:mainfrom
ratmice:cli_session

Conversation

@ratmice

@ratmice ratmice commented Jul 8, 2026

Copy link
Copy Markdown

Which issue does this PR close?

Closes #23386.

Rationale for this change

Customizing datafusion-cli with custom udf or calling register_* currently requires copying all of main.rs,
then dealing with the workspace dependencies required to build it, and change it to link against the datafusion-cli library.

If one just wants an example cli without much hassle and maintenance, it is a bit much when you hope to just add a couple lines.

What changes are included in this PR?

The first patch moves a lot of main_inner to a new struct ClientSession, that then makes it a lot simpler.
Included is an example of registering a cli-custom-udf.rs.

The second patch just reformats the code with rustfmt to deal with the addition of indentation by moving
things into an impl CliSession.

Are these changes tested?

I tested these by running the datafusion-cli and running select hello(1); in the new example.
Otherwise this doesn't change any existing library code.

Are there any user-facing changes?

ratmice added 2 commits July 8, 2026 04:49
Unformatted to ease review.

This also adds an example of reusing the main datafusion-cli while
registering custom udf.
@ratmice

ratmice commented Jul 8, 2026

Copy link
Copy Markdown
Author

Hmm, I'm uncertain how to make this patch look nice in the github review UI. It looks okay in my local git history viewer, but afaict github really wants to display them squashed?

@ratmice

ratmice commented Jul 8, 2026

Copy link
Copy Markdown
Author

After sleeping on it, there are definitely a few things this patch could do better.

  1. Hard coding of "DataFusion CLI {DATAFUSION_CLI_VERSION}"
  2. Calling Args::parse which will exit instead of Args::try_parse or Args::try_parse_from which will return a Result.
  3. Probably should just bite the bullet and make Args public, extending the existing example to use the command line arguments for configuring it's CliSessionContext, instead of calling SessionContext::new().
  4. That'll require taking a CliSessionContext parameter, and probably taking ownership of PrintOptions from a CliSession, perhaps the fields of CliSession should just be pub?.

Mostly fixed in 249072e, except the first point which I'm not sure needs to be done and seems minor to me.

@ratmice

ratmice commented Jul 8, 2026

Copy link
Copy Markdown
Author

So the only thing I'm a bit uncertain/hesitant about (I'm really not all that familiar with clap), is whether this will work
for anyone who wants to extend the arguments using #[command(flatten)], adding their own command line options, but reuse the datafusion-cli ones. I kind of assume that use case will need additional changes with something like CliSession::from_clap_args(clap_args: entry_point::Args) -> Result<CliSession, CliError>, but uncertain and hesitate to implement it without anything trying to actually do that.

Edit: After thinking about, this seemed like I should just do this now as it seemed inevitable, and it's better if we don't have to break semver later to handle it. Fixed in e3d941a

@ratmice

ratmice commented Jul 9, 2026

Copy link
Copy Markdown
Author

Alright I think this has ended up in a reasonable state (modulo bike shedding), and I can't imagine anything else (crosses fingers), so I believe it should be ready for review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Registering custom udf in datafusion-cli could be easier

1 participant