Skip to content

fix(scan): reject missing column stats fields#759

Merged
wgtmac merged 1 commit into
apache:mainfrom
fallintoplace:fix/strict-schema-select
Jun 30, 2026
Merged

fix(scan): reject missing column stats fields#759
wgtmac merged 1 commit into
apache:mainfrom
fallintoplace:fix/strict-schema-select

Conversation

@fallintoplace

@fallintoplace fallintoplace commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • validate IncludeColumnStats() column names against the resolved snapshot schema during scan build
  • return a validation error when requested stats columns are missing instead of silently dropping them
  • leave Schema::Select() and regular scan projection semantics unchanged

Tests

  • pre-commit run --all-files
  • cmake --build build --target scan_test
  • ctest --test-dir build -R '^scan_test$' --output-on-failure

@fallintoplace fallintoplace changed the title Fail schema select on missing columns fix: fail schema select on missing columns Jun 18, 2026
@wgtmac

wgtmac commented Jun 29, 2026

Copy link
Copy Markdown
Member

Ah, I didn't notice this PR before my agent raised #786. By practice we should respect the 1st PR that works on the same issue. However I think this PR chose the wrong direction (changed the semantics of Schema::Select) and applied a more complex fix (relied on heavy call to Schema::Select for validation). Do you mind changing your PR to use the approach in #786? Then I can merge this one and close that one.

@fallintoplace

Copy link
Copy Markdown
Contributor Author

Thank you @wgtmac, I will push the commit in a few hours.

@fallintoplace fallintoplace force-pushed the fix/strict-schema-select branch from 8840174 to f14a1a9 Compare June 29, 2026 18:36
@fallintoplace fallintoplace changed the title fix: fail schema select on missing columns fix(scan): reject missing column stats fields Jun 29, 2026
@fallintoplace

Copy link
Copy Markdown
Contributor Author

Thanks @wgtmac, pushed the narrower version following #786’s approach.

This now validates missing IncludeColumnStats() columns during scan build and leaves Schema::Select() semantics unchanged. I also updated the PR title/description. CI is green now.

@wgtmac

wgtmac commented Jun 30, 2026

Copy link
Copy Markdown
Member

Thank you, @fallintoplace!

@wgtmac wgtmac merged commit 68ee0f4 into apache:main Jun 30, 2026
21 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.

2 participants