Skip to content

Add builder API for CreateExternalTable to reduce redundancy and make the code easier to understand #19039

Description

@alamb

Is your feature request related to a problem or challenge?

While reviewing #18971. I noticed many examples like this over the codbase

          let cmd = CreateExternalTable {
              name,
              location,
              file_type: "parquet".to_string(),
              schema: Arc::new(DFSchema::empty()),
              table_partition_cols: vec![],
              if_not_exists: false,
              or_replace: false,
              temporary: false,
              definition: None,
              order_exprs: vec![],
              unbounded: false,
              options: HashMap::new(),
              constraints: Constraints::default(),
              column_defaults: HashMap::new(),
          };

This is non ideal for several reasons:

  1. It is verbose (most of the fields are the defaults)
  2. It makes it hard to see what fields are actually overridden vs which are the defaults

Describe the solution you'd like

instead of having to provide values for all fields, most of which are the default values, I think it would be nice to have a well commented builder API

Describe alternatives you've considered

I suggest adding a CrateExternalTableBuilder and replace the above code it with something like this

  let cmd = CreateExternalTable::builder()
    .with_name(name)
    .with_location(location)
    .with_file_type("parquet")
    .with_schema(schema)
    .build()?

It may make sense to have required parameters be verified by the compiler like this

  let cmd = CreateExternalTable::builder(name, location, "parquet", schema)
    .build()?

Additional context

No response

Activity

  1. added theissue type on Dec 1, 2025
  2. alamb commented on Dec 1, 2025

    @alamb
    ContributorAuthor

    I think this is a good issue for someone who wants to learn how the code works and get a contribution

    I think an LLM can probably pound it out pretty fast too, so a large point of me writing this issue is to help others get some experience with the DataFusion codebase

  3. atifxcode commented on Dec 2, 2025

    @atifxcode

    Hi @alamb, I would like to work on this issue.

    I’ve reviewed the current CreateExternalTable struct and the use cases where many default fields are repeated. Adding a CreateExternalTableBuilder makes sense to simplify construction and make overridden fields clearer.

    Before I start, could you confirm two things:

    1. Should the builder enforce required fields through its constructor (e.g., name, location, file_type, schema)?
    2. Should the builder live in the same module as CreateExternalTable, or would you prefer placing it in a separate builder module?

    If this approach aligns with your expectations, I’m happy to proceed. Thanks!

  4. AryanBagade commented on Dec 3, 2025

    @AryanBagade
    Contributor

    Hey @alamb
    I have created a PR and implemented a builder API, following ur second approach (required parameters in the constructor)

    I went with:

    CreateExternalTable::builder(name, location, file_type,
    schema)
        .with_options(options)
        .build()

    This enforces the 4 core required fields at compile-time, while optional fields can be set via .with_*() methods (following the pattern I saw in DataFusionErrorBuilder).

  5. added a commit that references this issue on Dec 5, 2025
    2b05b09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions