Skip to content

Fix nested field filtering#837

Merged
changhiskhan merged 3 commits intomainfrom
changhiskhan/filter-nested-field
May 10, 2023
Merged

Fix nested field filtering#837
changhiskhan merged 3 commits intomainfrom
changhiskhan/filter-nested-field

Conversation

@changhiskhan
Copy link
Copy Markdown
Contributor

This is on top of #836 , just split out for easier review.

Basically in the Take node, we do a.merge(b) for schema but b.merge(a) for RecordBatch.
So then in Projection, we get a schema that has struct_field with subfields f1 and f2, but the data has struct_field with subfields f2 and f1.

Comment thread rust/src/io/exec/projection.rs Outdated
.then(|(batch, schema)| async move {
let batch = batch?;
batch.project_by_schema(schema.as_ref())
let output_batch = batch.project_by_schema(schema.as_ref());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this was for debugging? can revert it back?

Comment thread rust/src/io/exec/take.rs
.take_rows(row_ids.values(), &extra)
.await?
.merge(&batch)?
batch.merge(&dataset.take_rows(row_ids.values(), &extra).await?)?
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what is the purpose to change the order of merge?

@changhiskhan changhiskhan force-pushed the changhiskhan/filter-nested-field branch from ca8927b to 0dc7382 Compare May 10, 2023 00:06
@changhiskhan changhiskhan merged commit c1c4599 into main May 10, 2023
@changhiskhan changhiskhan deleted the changhiskhan/filter-nested-field branch May 10, 2023 00:59
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