Skip to content

Conversation

@gene-bordegaray
Copy link
Contributor

@gene-bordegaray gene-bordegaray commented Oct 25, 2025

Which issue does this PR close?

Rationale for this change

The information_schema.views does not have display WITH ORDER for the definition of a table.

What changes are included in this PR?

Added condition for writing WITH ORDER for CreateExternalTable.

Are these changes tested?

Did not add tests for this functionality as not other display functionality has tests and seems like a separate PR would be appropriate if this is needed.

This was tested manually with:

In datafusion-cli

-- Not sorted
CREATE EXTERNAL TABLE  dimension_csv
STORED AS CSV 
LOCATION '/path/to/the/attached/dimension_1.csv'
OPTIONS ('format.has_header' 'true');

-- Sorted
CREATE EXTERNAL TABLE  dimension_csv_sorted
STORED AS CSV 
WITH ORDER (env, service, host)
LOCATION '/path/to/the/attached/dimension_1.csv'
OPTIONS ('format.has_header' 'true');

Then running:

select * from information_schema.views;

With link to data: dimension_1.csv

Are there any user-facing changes?

Yes, improves the information_schema.views display to include WITH ORDER

@github-actions github-actions bot added the sql SQL Planner label Oct 25, 2025
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.

Thanks @gene-bordegaray !

Can you please add some tests, ideally in sqllogicteset format?

Here are the instructions: https://github.com/apache/datafusion/tree/main/datafusion/sqllogictest

Ideally you should be able to extend https://github.com/apache/datafusion/tree/main/datafusion/sqllogictest/test_files/information_schema.slt

cc @NGA-TRAN as the filer of the request

@github-actions github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Oct 26, 2025
@gene-bordegaray
Copy link
Contributor Author

yes no problem @alamb , glad to start helping!

Just added some tests and will cc @NGA-TRAN

Copy link
Contributor

@NGA-TRAN NGA-TRAN left a comment

Choose a reason for hiding this comment

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

Looks great. You may want to add one more test suggested by @2010YOUY01 . Thanks @gene-bordegaray

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.

@alamb alamb added this pull request to the merge queue Oct 28, 2025
Merged via the queue into apache:main with commit bea4b68 Oct 28, 2025
28 checks passed
tobixdev pushed a commit to tobixdev/datafusion that referenced this pull request Nov 2, 2025
## Which issue does this PR close?

<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax. For example
`Closes apache#123` indicates that this PR will close issue apache#123.
-->

- Closes apache#18267.
/cc @NGA-TRAN 
## Rationale for this change

<!--
Why are you proposing this change? If this is already explained clearly
in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand
your changes and offer better suggestions for fixes.
-->
The `information_schema.views` does not have display `WITH ORDER` for
the definition of a table.

## What changes are included in this PR?

<!--
There is no need to duplicate the description in the issue here but it
is sometimes worth providing a summary of the individual changes in this
PR.
-->

Added condition for writing `WITH ORDER` for CreateExternalTable.

## Are these changes tested?

<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code

If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
-->
Did not add tests for this functionality as not other display
functionality has tests and seems like a separate PR would be
appropriate if this is needed.

This was tested manually with:

In `datafusion-cli`

```
-- Not sorted
CREATE EXTERNAL TABLE  dimension_csv
STORED AS CSV 
LOCATION '/path/to/the/attached/dimension_1.csv'
OPTIONS ('format.has_header' 'true');

-- Sorted
CREATE EXTERNAL TABLE  dimension_csv_sorted
STORED AS CSV 
WITH ORDER (env, service, host)
LOCATION '/path/to/the/attached/dimension_1.csv'
OPTIONS ('format.has_header' 'true');
```
Then running:
```
select * from information_schema.views;
```
With link to data:
[dimension_1.csv](https://github.com/user-attachments/files/23124138/dimension_1.csv)
## Are there any user-facing changes?

Yes, improves the information_schema.views display to include `WITH
ORDER`
<!--
If there are user-facing changes then we may require documentation to be
updated before approving the PR.
-->

<!--
If there are any breaking changes to public APIs, please add the `api
change` label.
-->
codetyri0n pushed a commit to codetyri0n/datafusion that referenced this pull request Nov 11, 2025
## Which issue does this PR close?

<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax. For example
`Closes apache#123` indicates that this PR will close issue apache#123.
-->

- Closes apache#18267.
/cc @NGA-TRAN 
## Rationale for this change

<!--
Why are you proposing this change? If this is already explained clearly
in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand
your changes and offer better suggestions for fixes.
-->
The `information_schema.views` does not have display `WITH ORDER` for
the definition of a table.

## What changes are included in this PR?

<!--
There is no need to duplicate the description in the issue here but it
is sometimes worth providing a summary of the individual changes in this
PR.
-->

Added condition for writing `WITH ORDER` for CreateExternalTable.

## Are these changes tested?

<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code

If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
-->
Did not add tests for this functionality as not other display
functionality has tests and seems like a separate PR would be
appropriate if this is needed.

This was tested manually with:

In `datafusion-cli`

```
-- Not sorted
CREATE EXTERNAL TABLE  dimension_csv
STORED AS CSV 
LOCATION '/path/to/the/attached/dimension_1.csv'
OPTIONS ('format.has_header' 'true');

-- Sorted
CREATE EXTERNAL TABLE  dimension_csv_sorted
STORED AS CSV 
WITH ORDER (env, service, host)
LOCATION '/path/to/the/attached/dimension_1.csv'
OPTIONS ('format.has_header' 'true');
```
Then running:
```
select * from information_schema.views;
```
With link to data:
[dimension_1.csv](https://github.com/user-attachments/files/23124138/dimension_1.csv)
## Are there any user-facing changes?

Yes, improves the information_schema.views display to include `WITH
ORDER`
<!--
If there are user-facing changes then we may require documentation to be
updated before approving the PR.
-->

<!--
If there are any breaking changes to public APIs, please add the `api
change` label.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sql SQL Planner sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

View should display table sort order when present

4 participants