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

feat(engine): handle target.* #933

Merged
merged 7 commits into from
Nov 21, 2024
Merged

feat(engine): handle target.* #933

merged 7 commits into from
Nov 21, 2024

Conversation

grieve54706
Copy link
Contributor

Handle the sql with dot all like SELECT table.* FROM table and SELECT t.* FROM table AS t.

@github-actions github-actions bot added ibis python Pull requests that update Python code labels Nov 20, 2024
@goldmedal
Copy link
Contributor

Thanks @grieve54706. Could you add some test for the model with calculated fields and the SQL with a join statement?

select a.* from a join b on a.c1 = b.c1

@grieve54706 grieve54706 added this to the 0.12.1 milestone Nov 21, 2024
Comment on lines 151 to 161
def test_query_with_dot_all(manifest_str, postgres: PostgresContainer):
connection_info = _to_connection_info(postgres)
response = client.post(
url=f"{base_url}/query",
params={"limit": 1},
json={
"connectionInfo": connection_info,
"manifestStr": manifest_str,
"sql": 'SELECT "o".* FROM "Orders" AS "o"',
},
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add the end-to-end SQL test for the calculated field and join syntax? Maybe we should add tested in TestDynamicFields, too. We need to make sure the model-generating behavior is fine in WrenSqlRewrite.

Copy link
Contributor

@goldmedal goldmedal left a comment

Choose a reason for hiding this comment

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

Thanks @grieve54706 it looks nice to me.

@goldmedal goldmedal merged commit e23469d into main Nov 21, 2024
7 checks passed
@goldmedal goldmedal deleted the feature/handle-targe-dot-all branch November 21, 2024 05:25
grieve54706 added a commit that referenced this pull request Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ibis python Pull requests that update Python code v1-engine-changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants