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 _execute_select and filter in the Query class. #2363

Merged
merged 1 commit into from
Aug 2, 2024

Conversation

jieguangzhou
Copy link
Collaborator

@jieguangzhou jieguangzhou commented Aug 1, 2024

Description

table.select('x', 'y').filter(table['x'] == 1 and table['y'] == 2))

Related Issues

Checklist

  • Is this code covered by new or existing unit tests or integration tests?
  • Did you run make unit_testing and make integration-testing successfully?
  • Do new classes, functions, methods and parameters all have docstrings?
  • Were existing docstrings updated, if necessary?
  • Was external documentation updated, if necessary?

Additional Notes or Comments

@jieguangzhou jieguangzhou marked this pull request as draft August 1, 2024 10:10
@jieguangzhou jieguangzhou force-pushed the feat/select-filter branch 5 times, most recently from d1d61bc to 8dcae9d Compare August 1, 2024 13:52
:param args: The arguments to filter by.
:param kwargs: Additional keyword arguments.
"""
query = self if self.parts else self.select()
Copy link
Collaborator Author

@jieguangzhou jieguangzhou Aug 1, 2024

Choose a reason for hiding this comment

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

Make sure the flavour of single filter query should be the select

Copy link
Collaborator

Choose a reason for hiding this comment

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

This function will conflict with the ibis implementation of .filter

Copy link
Collaborator

Choose a reason for hiding this comment

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

self.select() implies select * from table right?

I

Copy link
Collaborator Author

@jieguangzhou jieguangzhou Aug 2, 2024

Choose a reason for hiding this comment

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

We will find all the columns and fill in to the args.

And I removed this filter method in the base query class

Comment on lines +532 to +477
return self._get_chain_native_query(parent, self.parts, method)

def _get_chain_native_query(self, parent, parts, method='encode'):
try:
for part in parts:
if isinstance(part, str):
parent = getattr(parent, part)
continue
args = self._encode_or_unpack_args(
part[1], self.db, method=method, parent=parent
)
kwargs = self._encode_or_unpack_args(
part[2], self.db, method=method, parent=parent
)
parent = getattr(parent, part[0])(*args, **kwargs)
except Exception as e:
logging.error(f'Error in executing query, parts: {parts}')
raise e

Copy link
Collaborator Author

@jieguangzhou jieguangzhou Aug 1, 2024

Choose a reason for hiding this comment

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

Separating this method allows for easier future expansion of the query, enabling the query to flexibly construct the parts list without being tied to the expression.

In the future, all Query classes can be merged into one class, with Query only recording the expression and being independent the type of the database.

We can use Databackend to execute the Query

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, yes this is a possible approach.

@@ -437,26 +480,3 @@ def __call__(self, *args, **kwargs):
table = self.db.databackend.get_table_or_collection(self.table)
args = tuple(table.columns)
return super().__call__(*args, **kwargs)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Remove unuse codes

Comment on lines -583 to -584
class MongoOutputs(MongoQuery):
"""A query class for MongoDB outputs.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Use _execute_outputs to replace this class.

@jieguangzhou jieguangzhou marked this pull request as ready for review August 1, 2024 14:03
if not isinstance(item, slice):
return super().__getitem__(item)
raise TypeError('Query index must be a string or a slice')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The parent class does not define the getitem method.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes

@jieguangzhou jieguangzhou changed the title Add _execute_select in Query Add _execute_select and filter in the Query class. Aug 1, 2024
@jieguangzhou jieguangzhou force-pushed the feat/select-filter branch 5 times, most recently from fb2544b to 95900fa Compare August 2, 2024 07:58
@blythed blythed force-pushed the feat/select-filter branch from 95900fa to 07ab9d1 Compare August 2, 2024 10:01

Use aggregate to implement the outputs query
"""
def _get_filter_confitions(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

small typo

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@blythed blythed merged commit c193ade into superduper-io:main Aug 2, 2024
3 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.

[SIMPLIFY-2] Consider supporting only .select with filter for read queries and .insert
3 participants