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.

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

Feature: Add new formatter to export data to sql like mycli #1366

Merged
merged 4 commits into from
Sep 15, 2022

Conversation

astroshot
Copy link
Contributor

@astroshot astroshot commented Sep 9, 2022

Description

New formatter is added to export query result to sql-insert format and sql-update format like mycli:

For example:

PostgreSQL [email protected]:5432/test ➜ SELECT * FROM "user";
id  name     email                   phone      description  created_at                     updated_at
--  -------  ----------------------  ---------  -----------  -----------------------------  -----------------------------
1   Jackson  [email protected]  132454789               2022-09-09 19:44:32.712343+08  2022-09-09 19:44:32.712343+08
SELECT 1
Time: 0.028s

# Choose sql-insert output format
PostgreSQL [email protected]:5432/test ➜ \T sql-insert;
Changed table format to sql-insert
Time: 0.001s

# Then we have sql-insert output of query result
PostgreSQL [email protected]:5432/test ➜ SELECT * FROM "user";
INSERT INTO "user" ("id", "name", "email", "phone", "description", "created_at", "updated_at") VALUES
('1', 'Jackson', '[email protected]', '132454789', '', '2022-09-09 19:44:32.712343+08', '2022-09-09 19:44:32.712343+08')
;
SELECT 1
Time: 0.004s

# Choose sql-update format
PostgreSQL [email protected]:5432/test ➜ \T sql-update;
Changed table format to sql-update
Time: 0.000s
PostgreSQL [email protected]:5432/test ➜ SELECT * FROM "user";
UPDATE "user" SET
"name" = 'Jackson'
, "email" = '[email protected]'
, "phone" = '132454789'
, "description" = ''
, "created_at" = '2022-09-09 19:44:32.712343+08'
, "updated_at" = '2022-09-09 19:44:32.712343+08'
WHERE "id" = '1';
SELECT 1
Time: 0.004s

Checklist

  • I've added this contribution to the changelog.rst.
  • I've added my name to the AUTHORS file (or it's already there).
  • I installed pre-commit hooks (pip install pre-commit && pre-commit install), and ran black on my code.
  • Please squash merge this pull request (uncheck if you'd like us to merge as multiple commits)

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Sep 9, 2022

This pull request introduces 1 alert when merging b64b610 into abc03c5 - view on LGTM.com

new alerts:

  • 1 for Unused import

Body: New formatter is added, we can export query result to sql
insertion like mycli

==== End ====
@astroshot
Copy link
Contributor Author

astroshot commented Sep 9, 2022

I tested this code by pip install -e .. But I didn't do

pip install pre-commit && pre-commit install, and ran black on my code.`

So what do I need to do before this PR is reviewed?

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Sep 9, 2022

This pull request introduces 1 alert when merging 59ae529 into abc03c5 - view on LGTM.com

new alerts:

  • 1 for Unused import

@j-bennet
Copy link
Contributor

@astroshot We use black to format our code, and pre-commit to enforce it. The CI build will fail if the code is not black-ified.

@codecov-commenter
Copy link

codecov-commenter commented Sep 10, 2022

Codecov Report

Base: 84.15% // Head: 79.65% // Decreases project coverage by -4.49% ⚠️

Coverage data is based on head (773f1a7) compared to base (6884c29).
Patch coverage: 48.36% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1366      +/-   ##
==========================================
- Coverage   84.15%   79.65%   -4.50%     
==========================================
  Files          21       25       +4     
  Lines        2720     2954     +234     
==========================================
+ Hits         2289     2353      +64     
- Misses        431      601     +170     
Impacted Files Coverage Δ
pgcli/packages/parseutils/tables.py 97.67% <ø> (ø)
pgcli/packages/sqlcompletion.py 97.67% <ø> (ø)
pgcli/pgtoolbar.py 31.57% <0.00%> (+0.15%) ⬆️
pgcli/pyev.py 15.38% <15.38%> (ø)
pgcli/explain_output_formatter.py 46.15% <46.15%> (ø)
pgcli/key_bindings.py 52.94% <50.00%> (-0.19%) ⬇️
pgcli/pgexecute.py 83.22% <87.14%> (+2.86%) ⬆️
pgcli/auth.py 91.30% <91.30%> (ø)
pgcli/packages/formatter/sqlformatter.py 93.18% <93.18%> (ø)
pgcli/main.py 78.69% <93.33%> (+0.15%) ⬆️
... and 7 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@j-bennet
Copy link
Contributor

@astroshot Your new formatter could be useful to more than just pcli. What do you think about moving it into cli_helpers?

@astroshot
Copy link
Contributor Author

@j-bennet OK, I'll install black and recommit my code later.

The reason I didn't put this formatter to cli_helpers now is that different database use different delimiters to disambiguate table name from SQL keywords. For example, postgres uses " and mysql uses back quote ` .

For example, in postgres, insertion sql is:

INSERT INTO "user" ("id", "name", "email", "phone", "description", "created_at", "updated_at") VALUES
('1', 'Jackson', '[email protected]', '132454789', '', '2022-09-09 19:44:32.712343+08', '2022-09-09 19:44:32.712343+08')
;

And in myql, it is:

INSERT INTO `user` (`id`, `name`, `email`, `phone`, `description`, `created_at`, `updated_at`) VALUES
('1', 'Jackson', '[email protected]', '132454789', '', '2022-09-09 19:44:32.712343+08', '2022-09-09 19:44:32.712343+08')
;

Yet I'm not familiar with oracle and Mircosoft SQL Server.

So I'd like to merge this PR so that we can use it in pgcli as soon as possible. Then I'll try to move this formatter to cli_helpers, and use different delimiters for different database type passed from pgcli (or mycli and other clis), and it will take more time for fully test.
Do you think it's ok?

@j-bennet
Copy link
Contributor

@astroshot

The reason I didn't put this formatter to cli_helpers now is that different database use different delimiters to disambiguate table name from SQL keywords. For example, postgres uses " and mysql uses back quote ` .

You could make your formatter configurable.

So I'd like to merge this PR so that we can use it in pgcli as soon as possible. Then I'll try to move this formatter to cli_helpers, and use different delimiters for different database type passed from pgcli (or mycli and other clis), and it will take more time for fully test.

Sounds good to me.

Before merging, this new adapter needs some unit tests. Please add some, they don't have to be extensive.

@astroshot
Copy link
Contributor Author

@astroshot

The reason I didn't put this formatter to cli_helpers now is that different database use different delimiters to disambiguate table name from SQL keywords. For example, postgres uses " and mysql uses back quote ` .

You could make your formatter configurable.

So I'd like to merge this PR so that we can use it in pgcli as soon as possible. Then I'll try to move this formatter to cli_helpers, and use different delimiters for different database type passed from pgcli (or mycli and other clis), and it will take more time for fully test.

Sounds good to me.

Before merging, this new adapter needs some unit tests. Please add some, they don't have to be extensive.

Sure, I'll commit unit tests later.

@astroshot
Copy link
Contributor Author

@astroshot

The reason I didn't put this formatter to cli_helpers now is that different database use different delimiters to disambiguate table name from SQL keywords. For example, postgres uses " and mysql uses back quote ` .

You could make your formatter configurable.

So I'd like to merge this PR so that we can use it in pgcli as soon as possible. Then I'll try to move this formatter to cli_helpers, and use different delimiters for different database type passed from pgcli (or mycli and other clis), and it will take more time for fully test.

Sounds good to me.
Before merging, this new adapter needs some unit tests. Please add some, they don't have to be extensive.

Sure, I'll commit unit tests later.

@j-bennet Hi, I have committed unit tests, is there any more work to do before merging?

@j-bennet
Copy link
Contributor

You also want to add the new supported formats to pgclirc.

@astroshot
Copy link
Contributor Author

You also want to add the new supported formats to pgclirc.

@j-bennet New formatters have been added to pgclirc 😁

@j-bennet j-bennet merged commit c0e1081 into dbcli:main Sep 15, 2022
@j-bennet
Copy link
Contributor

@astroshot Your change is now released as part of 3.5.0.

@astroshot
Copy link
Contributor Author

@astroshot Your change is now released as part of 3.5.0.

@j-bennet Thanks 😁

@astroshot astroshot deleted the feature-export-sql branch September 17, 2022 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants