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 FILTER_DISTINCT function. #589

Merged
merged 6 commits into from
May 19, 2022

Conversation

dlurton
Copy link
Member

@dlurton dlurton commented Apr 29, 2022

This PR was originally part of #582 and is based on #588, which must be merged first.

This function is used by the planner to implement SELECT DISTINCT functionality.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

- AST->logical
- logical->resolved
- lesolved->physical.

Not yet integrated with anything--that will come in a future commit.
@dlurton dlurton force-pushed the physical-plan-staging-planner-passes branch from ac6155a to 48a003e Compare May 10, 2022 23:03
@dlurton dlurton force-pushed the physical-plan-staging-filter-distinct branch from 405ae74 to befac52 Compare May 11, 2022 00:14
@dlurton dlurton requested a review from alancai98 May 11, 2022 00:56
@dlurton dlurton marked this pull request as ready for review May 11, 2022 00:56

override fun callWithRequired(session: EvaluationSession, required: List<ExprValue>): ExprValue {
val argument = required.first()
// We cannot use a [HashSet] here because [ExprValue] does not implement .equals() and .hashCode()
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be useful for PartiQL to have the equivalent of Ion Hash for use cases like this.

Base automatically changed from physical-plan-staging-planner-passes to physical-plan-staging May 19, 2022 20:51
@dlurton dlurton merged commit 6fd6cc5 into physical-plan-staging May 19, 2022
@dlurton dlurton deleted the physical-plan-staging-filter-distinct branch May 19, 2022 21:35
dlurton added a commit that referenced this pull request May 27, 2022
Merges the following PRs to `main`:

- #583 
- #584 
- #587 
- #588 
- #589 
- #609 
- #590 
- #592 

Also, this adds a TODO comment (ea40e4a, requested by @am357 in offline chat) and excludes `PlannerPipeline` from the new aggregate tests which were pulled in when merging from `main` and were not part of the other PRs (34025b3).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
RCHowell pushed a commit that referenced this pull request Jun 1, 2022
Merges the following PRs to `main`:

- #583 
- #584 
- #587 
- #588 
- #589 
- #609 
- #590 
- #592 

Also, this adds a TODO comment (ea40e4a, requested by @am357 in offline chat) and excludes `PlannerPipeline` from the new aggregate tests which were pulled in when merging from `main` and were not part of the other PRs (34025b3).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
RCHowell pushed a commit that referenced this pull request Jun 1, 2022
Merges the following PRs to `main`:

- #583 
- #584 
- #587 
- #588 
- #589 
- #609 
- #590 
- #592 

Also, this adds a TODO comment (ea40e4a, requested by @am357 in offline chat) and excludes `PlannerPipeline` from the new aggregate tests which were pulled in when merging from `main` and were not part of the other PRs (34025b3).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
RCHowell pushed a commit that referenced this pull request Jun 1, 2022
Merges the following PRs to `main`:

- #583 
- #584 
- #587 
- #588 
- #589 
- #609 
- #590 
- #592 

Also, this adds a TODO comment (ea40e4a, requested by @am357 in offline chat) and excludes `PlannerPipeline` from the new aggregate tests which were pulled in when merging from `main` and were not part of the other PRs (34025b3).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
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.

2 participants