Skip to content

More fixes for parquet reader v3, enable it in stateless tests#87600

Merged
al13n321 merged 7 commits intomasterfrom
pqf
Oct 21, 2025
Merged

More fixes for parquet reader v3, enable it in stateless tests#87600
al13n321 merged 7 commits intomasterfrom
pqf

Conversation

@al13n321
Copy link
Member

@al13n321 al13n321 commented Sep 24, 2025 •
edited
Loading

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

This PR is stacked on top of #87220 , review only the commits starting from Fixes for parquet reader v3, enable it in stateless tests.

Closes #87509
Closes #87396

All stateless tests should pass now.

@clickhouse-gh
Copy link
Contributor

clickhouse-gh bot commented Sep 24, 2025 •
edited
Loading

Workflow [PR], commit [93fa266]

Summary: ❌

job_name test_name status info comment
Stateless tests (amd_binary, ParallelReplicas, s3 storage, parallel) failure
03658_joined_block_split_single_row_bytes FAIL cidb
03578_parallel_replicas_minicrawl FAIL cidb, flaky

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Sep 24, 2025
@divanik divanik self-assigned this Sep 24, 2025
@al13n321 al13n321 mentioned this pull request Sep 27, 2025
Merged
@EmeraldShift
Copy link
Contributor

EmeraldShift commented Sep 28, 2025 •
edited
Loading

Any chance these and all your other good fixes to pqv3 and index analysis can be backported to 25.8 LTS and 25.9-stable?

divanik
divanik reviewed Oct 2, 2025
View reviewed changes
Copy link
Member

@divanik divanik left a comment

Choose a reason for hiding this comment

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

Took a look partially, will review SchemaConverter a bit later

@al13n321 al13n321 added no-fast-tests Drops preliminary CI jobs: Style, clang-tidy, Fast Test ci-non-blocking Do not block pipeline on too many failed tests labels Oct 21, 2025
@al13n321
Copy link
Member Author

al13n321 commented Oct 21, 2025 •
edited
Loading

Is there a label to make CI run all tests? Does ci-non-blocking do that? Does no-fast-tests do that (except for fast tests)? EDIT: Yes, at least one of these seems to work.

@al13n321 al13n321 removed the no-fast-tests Drops preliminary CI jobs: Style, clang-tidy, Fast Test label Oct 21, 2025
@al13n321
Copy link
Member Author

al13n321 commented Oct 21, 2025

03578_parallel_replicas_minicrawl and ast fuzzer are flaky.

divanik
divanik reviewed Oct 21, 2025
View reviewed changes
divanik
divanik approved these changes Oct 21, 2025
View reviewed changes
src/Processors/Formats/Impl/Parquet/SchemaConverter.h
if (parquet_name.has_value())
*parquet_name += ".";
else
parquet_name.emplace();
Copy link
Member

@divanik divanik Oct 21, 2025

Choose a reason for hiding this comment

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

Shouldn't we copy the whole name here?

Copy link
Member Author

@al13n321 al13n321 Oct 21, 2025

Choose a reason for hiding this comment

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

The next line does that? Like, if it's the first component of the name then append it to empty string, without a '.'

@al13n321
Copy link
Member Author

al13n321 commented Oct 21, 2025

03658_joined_block_split_single_row_bytes and 03578_parallel_replicas_minicrawl are flaky.

@al13n321 al13n321 enabled auto-merge October 21, 2025 15:31
@al13n321 al13n321 added this pull request to the merge queue Oct 21, 2025
Merged via the queue into master with commit 58971a6 Oct 21, 2025
121 of 123 checks passed
@al13n321 al13n321 deleted the pqf branch October 21, 2025 15:44
@robot-ch-test-poll4 robot-ch-test-poll4 added the pr-synced-to-cloud The PR is synced to the cloud repo label Oct 21, 2025
@george-larionov george-larionov mentioned this pull request Oct 24, 2025
Closed
@nikitamikhaylov nikitamikhaylov mentioned this pull request Oct 27, 2025
Merged
1 task
mkmkme pushed a commit to Altinity/ClickHouse that referenced this pull request Nov 13, 2025
@al13n321 @mkmkme
Merge pull request ClickHouse#87600 from ClickHouse/pqf
1308f7a 
More fixes for parquet reader v3, enable it in stateless tests
@mkmkme mkmkme mentioned this pull request Nov 13, 2025
Merged
25 tasks
Enmk added a commit to Altinity/ClickHouse that referenced this pull request Nov 25, 2025
@Enmk
Merge pull request #1138 from Altinity/backports/antalya-25.8/87600
fcb6ded 
Antalya 25.8 Backport of ClickHouse#87600 - More fixes for parquet reader v3, enable it in stateless tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@divanik divanik divanik approved these changes

Assignees

@divanik divanik

Labels

ci-non-blocking Do not block pipeline on too many failed tests 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

Milestone

No milestone

5 participants

@al13n321 @EmeraldShift @divanik @robot-ch-test-poll4 @svb-alt