Skip to content

Conversation

@jimexist
Copy link
Member

@jimexist jimexist commented Nov 5, 2021

Which issue does this PR close?

Closes #1250

Rationale for this change

What changes are included in this PR?

correct sql input

❯ select cos(2.0);
+------------------------------+
| Float64(-0.4161468365471424) |
+------------------------------+
| -0.4161468365471424          |
+------------------------------+
1 row in set. Query took 0.004 seconds.

invalid SQL

❯ from a join b select 9;  🤔 Invalid statement: sql parser error: Expected an SQL statement, found: from

correct command input

❯ \d
+---------------+--------------------+------------+------------+
| table_catalog | table_schema       | table_name | table_type |
+---------------+--------------------+------------+------------+
| datafusion    | information_schema | tables     | VIEW       |
| datafusion    | information_schema | columns    | VIEW       |
+---------------+--------------------+------------+------------+
2 rows in set. Query took 0.004 seconds.

empty statement

❯ ;  🤔 You entered an empty statement

multiple statement

❯ select 1; select 2;  🤔 You entered more than one statement

multiline editing

❯ select
1



+

2



;
+----------+
| Int64(3) |
+----------+
| 3        |
+----------+
1 row in set. Query took 0.002 seconds.

edge case with comment

❯ -- select 1;  🤔 You entered an empty statement
❯ select -- 1
2


;
+----------+
| Int64(2) |
+----------+
| 2        |
+----------+
1 row in set. Query took 0.001 seconds.
❯


select ';     '









;
+----------------+
| Utf8(";     ") |
+----------------+
| ;              |
+----------------+
1 row in set. Query took 0.001 seconds.
❯ select -- select
-- select
-- --
1;
+----------+
| Int64(1) |
+----------+
| 1        |
+----------+
1 row in set. Query took 0.001 seconds.

Are there any user-facing changes?

@alamb
Copy link
Contributor

alamb commented Nov 6, 2021

I started playing around with this locally -- it is very cool. Going to review it now more carefully

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.

I read the code and played around with this and it was very cool 👍

Can't wait to see something like SQL autocompletion 😍

I wonder if it would be possible to write tests for this somehow -- not because it doesn't work now, but this is the kind of cool behavior that could easily get messed up if/ when we add new behaviors / auto complete

@jimexist jimexist merged commit 32528f1 into apache:master Nov 6, 2021
@jimexist jimexist deleted the cli-editor branch November 6, 2021 12:04
@jimexist
Copy link
Member Author

jimexist commented Nov 6, 2021

I read the code and played around with this and it was very cool 👍

Can't wait to see something like SQL autocompletion 😍

I wonder if it would be possible to write tests for this somehow -- not because it doesn't work now, but this is the kind of cool behavior that could easily get messed up if/ when we add new behaviors / auto complete

agreed, however i cannot find easy way to unit test the editor as this requires user input simulation. will try later if i have more time.

@houqp houqp added the enhancement New feature or request label Nov 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

datafusion cli to add sql statement validation editor support

3 participants