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

Add support of SAMPLE ClickHouse clause #707

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

YiuRULE
Copy link
Contributor

@YiuRULE YiuRULE commented Oct 12, 2022

No description provided.

@YiuRULE YiuRULE requested a review from a team as a code owner October 12, 2022 08:30
@coveralls
Copy link

coveralls commented Oct 12, 2022

Coverage Status

coverage: 98.387% (+0.005%) from 98.382% when pulling 7801d05 on Kadiska:gt/clickhouse_sample into f46ec85 on kayak:master.

@AzisK
Copy link

AzisK commented Sep 29, 2023

Could you pull from the master branch so that the newest CI test kick in?

@ns-gtassery ns-gtassery force-pushed the gt/clickhouse_sample branch 3 times, most recently from 115c4c1 to e8c2c5d Compare October 2, 2023 04:17
@YiuRULE
Copy link
Contributor Author

YiuRULE commented Oct 2, 2023

done, it has been rebase to master

@@ -781,6 +781,16 @@ def drop_view(self, view: str) -> "ClickHouseDropQueryBuilder":
class ClickHouseQueryBuilder(QueryBuilder):
QUERY_CLS = ClickHouseQuery

def __init__(self, **kwargs) -> None:
super().__init__(self, as_keyword=True)
Copy link

Choose a reason for hiding this comment

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

is there no need to send other kwargs to the super().__init__?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think my previous code was indeed wrong, it has been corrected on the latest revision, thanks for bringing up the issue

Copy link

@AzisK AzisK left a comment

Choose a reason for hiding this comment

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

I have not had a chance to use Clickhouse in my life but the code looks good. Thanks for your work

@AzisK AzisK merged commit 44dcc9d into kayak:master Oct 10, 2023
5 checks passed
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.

4 participants