Skip to content

Fix simple parquet reader v3 assert on bool column min/max#87095

Merged
alexey-milovidov merged 3 commits intomasterfrom
boolwark
Oct 19, 2025
Merged

Fix simple parquet reader v3 assert on bool column min/max#87095
alexey-milovidov merged 3 commits intomasterfrom
boolwark

Conversation

@al13n321
Copy link
Copy Markdown
Member

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Closes #87078

Some code assigned input_signed = 1, some other code didn't expect that value.

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Sep 14, 2025

Workflow [PR], commit [fbcdf34]

Summary:

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Sep 14, 2025
UInt64 val = 0;
switch (input_size)
{
case 1: val = unalignedLoad<UInt8>(data.data()); break;
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.

Should you add the 2-byte (ie UInt16) case as well?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's not used since the only int parquet physical types are BOOLEAN, INT32, and INT64. But ok, added just in case the reader of the code has OCD :)

@alexey-milovidov alexey-milovidov self-assigned this Oct 18, 2025
@alexey-milovidov alexey-milovidov added this pull request to the merge queue Oct 19, 2025
Merged via the queue into master with commit 14a656a Oct 19, 2025
123 checks passed
@alexey-milovidov alexey-milovidov deleted the boolwark branch October 19, 2025 03:45
@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Oct 19, 2025
mkmkme pushed a commit to Altinity/ClickHouse that referenced this pull request Jan 7, 2026
Fix simple parquet reader v3 assert on bool column min/max
zvonand added a commit to Altinity/ClickHouse that referenced this pull request Jan 15, 2026
Antalya 25.8 Backport of ClickHouse#87095 - Fix simple parquet reader v3 assert on bool column min/max
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Logical error: 'false' with Parquet reader v3

4 participants