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

bug: Filter predicate is duplicated in TableScanBuilder and TableScan 🤦🏼‍♂️ #407

Closed
sdd opened this issue Jun 18, 2024 · 2 comments · Fixed by #414
Closed

bug: Filter predicate is duplicated in TableScanBuilder and TableScan 🤦🏼‍♂️ #407

sdd opened this issue Jun 18, 2024 · 2 comments · Fixed by #414
Labels
bug Something isn't working good first issue Good for newcomers
Milestone

Comments

@sdd
Copy link
Contributor

sdd commented Jun 18, 2024

Somehow, between my PR and @viirya's PR getting merged, we've ended up with the filter predicate being duplicated in the TableScanBuilder and the TableScan.

See predicates and filter:

predicates: Option<Predicate>,
snapshot_id: Option<i64>,
batch_size: Option<usize>,
case_sensitive: bool,
filter: Option<Predicate>,

/// Specifies a predicate to use as a filter
pub fn with_filter(mut self, predicate: Predicate) -> Self {
// calls rewrite_not to remove Not nodes, which must be absent
// when applying the manifest evaluator
self.filter = Some(predicate.rewrite_not());
self
}

/// Add a predicate to the scan. The scan will only return rows that match the predicate.
pub fn filter(mut self, predicate: Predicate) -> Self {
self.predicates = Some(predicate);
self
}

And here in TableScan, bound_predicates and filter:

bound_predicates: Option<BoundPredicate>,
schema: SchemaRef,
batch_size: Option<usize>,
case_sensitive: bool,
filter: Option<Arc<Predicate>>,

The predicate pushdown uses one of these but the file plan uses another, which caused a lot of confusion when I was trying to track down why my queries were misbehaving!

@sdd
Copy link
Contributor Author

sdd commented Jun 18, 2024

@liurenjie1024 can you add the good first issue label please?

@liurenjie1024 liurenjie1024 added the good first issue Good for newcomers label Jun 19, 2024
@liurenjie1024
Copy link
Contributor

@liurenjie1024 can you add the good first issue label please?

Sure, done.

@liurenjie1024 liurenjie1024 changed the title Filter predicate is duplicated in TableScanBuilder and TableScan 🤦🏼‍♂️ bug: Filter predicate is duplicated in TableScanBuilder and TableScan 🤦🏼‍♂️ Jun 19, 2024
@liurenjie1024 liurenjie1024 added the bug Something isn't working label Jun 19, 2024
@liurenjie1024 liurenjie1024 added this to the 0.3.0 Release milestone Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants