-
Notifications
You must be signed in to change notification settings - Fork 478
feat: add union query support #2146
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
Changes from all commits
ee4ca6a
483f720
7d761b8
1484a9b
808c302
3e4dee7
9080bfb
18dfd14
852faa0
da59410
e96065b
7618481
ba5973e
8f6a104
d64f5b7
709ddfa
dbbed50
a1accfd
211bbfe
7c1225a
f8dd7c8
074a61e
7ddbc5e
7cd1817
9d541c2
5f4ad80
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -154,6 +154,33 @@ async def execute_select( | |
| await self._execute_prefetch_queries(instance_list) | ||
| return instance_list | ||
|
|
||
| async def execute_union( | ||
| self, sql: str, app_field: str, model_field: str, models: set[type[Model]] | ||
| ) -> list: | ||
| _, raw_results = await self.db.execute_query(sql) | ||
| instance_list = [] | ||
|
|
||
| for row_idx, row in enumerate(raw_results): | ||
| if row_idx != 0 and row_idx % CHUNK_SIZE == 0: | ||
| # Forcibly yield to the event loop to avoid blocking the event loop | ||
| # when selecting a large number of rows | ||
| await asyncio.sleep(0) | ||
|
|
||
| for model in models: | ||
| if ( | ||
| model._meta.app == row[app_field] | ||
| and model._meta._model.__name__ == row[model_field] | ||
| ): | ||
| row = { | ||
| field: row[field] | ||
| for field in row.keys() | ||
| if field not in [model_field, app_field] | ||
| } | ||
| instance_list.append(model._init_from_db(**row)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is not good idea to pass row like that, with extra values of app field and model field. I think we should strip them. I understand that it works now like that, but it seems fragile and can interfere with actual fields declared on model. Maybe even consider making these special field names more unique, to lower chance of overlapping with actual fields (maybt "_*" prefix?)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what you mean. We add 2 annotation fields: I can change the field names to
What do you mean by that?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are passing these field values inside model init, I think we shouldn't do it, we should pop them from row dict
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh ok, it makes sense. Addressed in 9d541c2 |
||
| break | ||
|
|
||
| return instance_list | ||
|
|
||
| def _prepare_insert_columns( | ||
| self, include_generated: bool = False | ||
| ) -> tuple[list[str], list[str]]: | ||
|
|
||
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.
MSSql limit syntax is different but it's not currently supported in
_SetOperationwhich has its own implementation of_limit_sql()instead of using the dialect SQL. I guess we can leave it as a TODO for laterhttps://github.com/tortoise/pypika-tortoise/blob/378f6145f7529d88fa58510851d80454b742406e/pypika_tortoise/queries.py#L672