Skip to content

Allow specifying --query multiple times in the command line#50357

Closed
Ziy1-Tan wants to merge 4 commits intoClickHouse:masterfrom
Ziy1-Tan:queries
Closed

Allow specifying --query multiple times in the command line#50357
Ziy1-Tan wants to merge 4 commits intoClickHouse:masterfrom
Ziy1-Tan:queries

Conversation

@Ziy1-Tan
Copy link
Contributor

@Ziy1-Tan Ziy1-Tan commented May 30, 2023 •
edited by rschu1ze
Loading

Closes #49970

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Allow specifying --query multiple times for clickhouse-local and clickhouse-client.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@evillique evillique self-assigned this May 30, 2023
@robot-clickhouse robot-clickhouse added the pr-improvement Pull request with some product improvements label May 31, 2023
@robot-clickhouse
Copy link
Member

robot-clickhouse commented May 31, 2023 •
edited by robot-clickhouse-ci-2
Loading

This is an automated comment for commit f41f65a with description of existing statuses. It's updated for the latest CI running
The full report is available here
The overall status of the commit is 🔴 failure

Check nameDescriptionStatus
CI runningA meta-check that indicates the running CI. Normally, it's in success or pending state. The failed status indicates some problems with the PR🟢 success
Docs CheckBuilds and tests the documentation🟢 success
Fast testNormally this is the first check that is ran for a PR. It builds ClickHouse and runs most of stateless functional tests, omitting some. If it fails, further checks are not started until it is fixed. Look at the report to see which tests fail, then reproduce the failure locally as described here🔴 failure
Mergeable CheckChecks if all other necessary checks are successful🟢 success
Push to DockerhubThe check for building and pushing the CI related docker images to docker hub🟢 success
Style CheckRuns a set of checks to keep the code style clean. If some of tests failed, see the related log from the report🟢 success

@evillique evillique added the can be tested Allows running workflows for external contributors label May 31, 2023
evillique
evillique reviewed May 31, 2023
View reviewed changes
evillique
evillique reviewed May 31, 2023
View reviewed changes
@Ziy1-Tan
Improve docs
f41f65a 
Signed-off-by: Ziy1-Tan <[email protected]>
@Ziy1-Tan
Copy link
Contributor Author

Ziy1-Tan commented May 31, 2023

cc @ucasfl @rschu1ze

rschu1ze
rschu1ze reviewed May 31, 2023
View reviewed changes
Copy link
Member

@rschu1ze rschu1ze left a comment

Choose a reason for hiding this comment

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

(took a look despite @Avogar being assigned already because @Ziy1-Tan nudged me)

tests/queries/0_stateless/02771_specify_query_multiple_times.sh
. "$CURDIR"/../shell_config.sh

$CLICKHOUSE_LOCAL --query "SELECT 101"
$CLICKHOUSE_LOCAL --query "SELECT 202" --query "SELECT 303"
Copy link
Member

@rschu1ze rschu1ze May 31, 2023

Choose a reason for hiding this comment

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

./clickhouse-local --query "select 1" --query "select 2;select 1;" produces

1
2
1

even if option --multiquery is not passed. That's kind of unexpected, the help for --multiquery says "If specified, multiple queries separated by semicolons can be listed after --query.".

EDIT: It is even more strange:

  • ./clickhouse-local --query "select 1; select 2" works (without --multiquery)
  • ./clickhouse-client --query "select 1; select 2" throws

EDIT of the EDIT: Interesting, this behavior in clickhouse-local and clickhouse-client already existed before. So this PR didn't cause this. But I am not sure if this is a bug or desired, at least clickhouse-local --help currently shows wrong information about --multiquery. @Ziy1-Tan Totally optional, but it would be great if you could check briefly if clickhouse-local could behave in the same way as clickhouse-client.

@alexey-milovidov
Copy link
Member

alexey-milovidov commented Sep 3, 2023

@Ziy1-Tan, tests have failed. Let's continue...

@rschu1ze rschu1ze mentioned this pull request Sep 4, 2023
Merged
@rschu1ze
Copy link
Member

rschu1ze commented Sep 4, 2023

I continued + finished the PR in #54249 as I had trouble pushing to @Ziy1-Tan 's repo. @Ziy1-Tan Thanks for the contribution!

@rschu1ze rschu1ze closed this Sep 4, 2023
@rschu1ze rschu1ze mentioned this pull request Sep 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@evillique evillique evillique left review comments

@rschu1ze rschu1ze rschu1ze left review comments

Assignees

@evillique evillique

Labels

can be tested Allows running workflows for external contributors pr-improvement Pull request with some product improvements

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow specifying --query multiple times in the command line

5 participants

@Ziy1-Tan @robot-clickhouse @alexey-milovidov @rschu1ze @evillique