Skip to content

fix(rust/sedona-pointcloud): projection regression#825

Merged
paleolimbot merged 1 commit into
apache:mainfrom
b4l:projection
May 11, 2026
Merged

fix(rust/sedona-pointcloud): projection regression#825
paleolimbot merged 1 commit into
apache:mainfrom
b4l:projection

Conversation

@b4l
Copy link
Copy Markdown
Contributor

@b4l b4l commented May 10, 2026

fixes #824 introduced in #797

@jiayuasu jiayuasu requested a review from paleolimbot May 11, 2026 01:14
Copy link
Copy Markdown
Member

@paleolimbot paleolimbot left a comment

Choose a reason for hiding this comment

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

Thank you! I am guessing this is a result of the DataFusion 42 updates I attempted 😬

Can we improve the test to catch a regression like this in the future?

@jiayuasu
Copy link
Copy Markdown
Member

jiayuasu commented May 11, 2026

A couple of suggestions before merging:

Tests

  • The previous projection test covered both .las and .laz; the rewrite only covers .las. Since LAZ goes through a separate decode path, it'd be good to keep the .laz assertion (or parameterize over both).
  • Assertions check column count + type of column 0, but not the actual data. The interesting failure mode in bug(rust/sedona-pointcloud): projection pushdown does not slice batches; non-prefix projections fail or return wrong data #824 was returning the right schema with the wrong column — same UInt8 shape, wrong values. An assert_eq! on a value (e.g. classification = 0 for the single point in extra.las) would lock that down.

Scope

  • LasSource::new(extension, table_schema: impl Into<TableSchema>) is an unrelated public-API change. Worth pulling out into a separate PR (or dropping) so this one is purely a fix.
  • The field reordering in LasSource and LasOpener is noisy in the diff without changing behaviour; reverting it would make the fix easier to cherry-pick.

@paleolimbot
Copy link
Copy Markdown
Member

@jiayuasu I'll move your comment to a follow-up ticket to track to improve the end-to-end testing for sedona-pointcloud...there are many more situations we need to check outside of the one you highlighted and I think we can merge this as a standalone improvement while we sort that out. Exposing this in Python is probably a good first step because we can use existing readers to independently check results.

@paleolimbot paleolimbot merged commit 0644b4c into apache:main May 11, 2026
17 checks passed
@b4l b4l deleted the projection branch May 11, 2026 15:28
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.

bug(rust/sedona-pointcloud): projection pushdown does not slice batches; non-prefix projections fail or return wrong data

3 participants