-
Notifications
You must be signed in to change notification settings - Fork 82
Remove redundent brackets for the join syntax #649
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
@@ -122,7 +179,7 @@ def test_query(self, clickhouse: ClickHouseContainer): | |||
) | |||
assert response.status_code == 200 | |||
result = response.json() | |||
assert len(result["columns"]) == len(self.manifest["models"][0]["columns"]) | |||
assert len(result["columns"]) == 9 |
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.
Why use a constant instead of an object reference?
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.
Because the model has calculated fields now, the number of columns will be different. In dynamic query mode, the calculated fields won't be selected when select all.
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.
Oh, I see. Thanks.
params={"limit": 1}, | ||
json={ | ||
"connectionInfo": connection_info, | ||
"manifestStr": self.manifest_str, | ||
"sql": 'SELECT name as customer_name FROM "Orders" join "Customer" on "Orders".custkey = "Customer".custkey WHERE custkey = 370 LIMIT 1', |
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.
Two limit could keep only one.
* remove redundant brackets * add relationship test for clickhouse * enable dynamic-query by default * fix checkstyle * fix dry plan test * remove redundant limit
Description
Close #647
The join relation with brackets is valid for Trino but isn't for Clickhouse.
Because
ibis
can't handle the syntax for Clickhouse, we have stopped generating the join syntax with brackets in this PR.Related Changed
dynamic-query
will be enable by default for wren-engine.