-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Implement DESCRIBE <table> #2642
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.
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
Conversation
liukun4515
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
alamb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contribution @LiuYuHui ! This is a great start.
I tried it out locally and it doesn't seem to work quite right yet:
(arrow_dev) alamb@MacBook-Pro-6:~/Software/arrow-datafusion/datafusion-cli$ cargo run
...
DataFusion CLI v8.0.0
❯ CREATE OR REPLACE TABLE y AS VALUES (1,2),(3,4);
+---------+---------+
| column1 | column2 |
+---------+---------+
| 1 | 2 |
| 3 | 4 |
+---------+---------+
2 rows in set. Query took 0.062 seconds.
❯ describe table;
0 rows in set. Query took 0.032 seconds.
❯ describe table y; 🤔 Invalid statement: sql parser error: Expected end of statement, found: y
Also, can you please add a test (perhaps you can use he example above).
It might be nice to add a test here:
https://github.com/LiuYuHui/arrow-datafusion/blob/describe_table/datafusion/core/tests/sql/information_schema.rs#L223
which you can run with a command like cargo test --test sql_integration
|
Hi @alamb, thanks for your review, currently this PR only implements |
🤦 -- no sorry @LiuYuHui I was mistaken on the syntax of I tried the correct syntax and it works great 👍 ❯ describe y;
+-------------+-----------+-------------+
| column_name | data_type | is_nullable |
+-------------+-----------+-------------+
| column1 | Int64 | YES |
| column2 | Int64 | YES |
+-------------+-----------+-------------+
2 rows in set. Query took 0.006 seconds.However, I do think this PR needs a test. Also, would it be possible to generate an error if the table didn't exist? For example, mysql does the following for a non existent table: mysql> describe ff;
ERROR 1146 (42S02): Table 'foo.ff' doesn't existBut this PR returns an empty row: ❯ describe table;
0 rows in set. Query took 0.020 seconds. |
|
@alamb, I have fixed the nonexistent table case and added the test, please take a look. |
alamb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR looks great now @LiuYuHui -- thank you very much
I tried it out locally too and 👍
Which issue does this PR close?
Closes #2606.
Rationale for this change
What changes are included in this PR?
When the user executes the
DESCRIBE <table>, the output will be the following.Are there any user-facing changes?
Does this PR break compatibility with Ballista?