Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support (order by / sort) for DataFrameWriteOptions #13874

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Support (order by / sort) for DataFrameWriteOptions #13874

wants to merge 3 commits into from
+223 −4

Conversation

zhuqi-lucas
Copy link
Contributor

@zhuqi-lucas zhuqi-lucas commented Dec 21, 2024 •
edited
Loading

Which issue does this PR close?

Closes #13873

Rationale for this change

DataFrameWriteOptions is missing an order by / sort by like available in SQL.

For sql we have the option to sort, for example:

You can use the WITH ORDER clause of the CREATE EXTERNAL TABLE if your data is already ordered

https://datafusion.apache.org/user-guide/sql/ddl.html#create-external-table

CREATE EXTERNAL TABLE test (
    c1  VARCHAR NOT NULL,
    c2  INT NOT NULL,
    c3  SMALLINT NOT NULL,
    c4  SMALLINT NOT NULL,
    c5  INT NOT NULL,
    c6  BIGINT NOT NULL,
    c7  SMALLINT NOT NULL,
    c8  INT NOT NULL,
    c9  BIGINT NOT NULL,
    c10 VARCHAR NOT NULL,
    c11 FLOAT NOT NULL,
    c12 DOUBLE NOT NULL,
    c13 VARCHAR NOT NULL
)
STORED AS CSV
-- this line tells DataFusion the data in the file is already ordered by (c2 ASC)
WITH ORDER (c2 ASC)
LOCATION '/path/to/aggregate_test_100.csv'
OPTIONS ('has_header' 'true');

But for writing my parquet or other format files, we don't support it

What changes are included in this PR?

Add the sort support.

Are these changes tested?

yes

Added unit testing.

Are there any user-facing changes?

Yes, we support new order option for DataFrameWriteOptions

@github-actions github-actions bot added the core Core DataFusion crate label Dec 21, 2024
alamb
alamb approved these changes Dec 22, 2024
View reviewed changes
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @zhuqi-lucas -- this PR is really nice in my mind -- it is well commented and well tested 🏅

cc @TheBuilderJR

datafusion/core/src/dataframe/mod.rs

assert_batches_eq!(
&[
"+---+---+",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a good test 👍

Dandandan
Dandandan reviewed Dec 22, 2024
View reviewed changes
datafusion/core/src/dataframe/parquet.rs
.sort(options.sort_by)?
.build()?
};

Copy link
Contributor

@Dandandan Dandandan Dec 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't LogicalPlanBuilder::copy_to have the ordering info as well?
If we write the info to a new table, do we know avoid sorting the table again? We should expose the ordering info to the newly created table for that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think copy_to inserts the data from whatever the input plan is (if there is a redundant sort the optimizer will remove it). Is that what you are asking?

It is an interesting question of how to communicate "this table is sorted" information for newly written files. Is this what you are talking about?

Copy link
Contributor

@Dandandan Dandandan Dec 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I mean that after this PR we seem we don't communicate "we are sorted" yet, which is I think would support usecases such as in the issue from @TheBuilderJR (write data sorted on a column, making following read query with order by on the same column fast).

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

@alamb alamb alamb approved these changes

@Dandandan Dandandan Dandandan left review comments

Assignees
No one assigned
Labels
core Core DataFusion crate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support (order by / sort) for DataFrameWriteOptions
3 participants
@zhuqi-lucas @Dandandan @alamb