Skip to content

Boolean support for parquet native reader#71055

Merged
vdimir merged 7 commits intoClickHouse:masterfrom
arthurpassos:support_for_boolean_parquet_native_reader
Oct 31, 2024
Merged

Boolean support for parquet native reader#71055
vdimir merged 7 commits intoClickHouse:masterfrom
arthurpassos:support_for_boolean_parquet_native_reader

Conversation

@arthurpassos
Copy link
Contributor

@arthurpassos arthurpassos commented Oct 25, 2024

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Boolean support for parquet native reader

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

Information about CI checks: https://clickhouse.com/docs/en/development/continuous-integration/

CI Settings (Only check the boxes if you know what you are doing):

  • Allow: All Required Checks
  • Allow: Stateless tests
  • Allow: Stateful tests
  • Allow: Integration Tests
  • Allow: Performance tests
  • Allow: All Builds
  • Allow: batch 1, 2 for multi-batch jobs
  • Allow: batch 3, 4, 5, 6 for multi-batch jobs
  • Exclude: Style check
  • Exclude: Fast test
  • Exclude: All with ASAN
  • Exclude: All with TSAN, MSAN, UBSAN, Coverage
  • Exclude: All with aarch64, release, debug
  • Run only fuzzers related jobs (libFuzzer fuzzers, AST fuzzers, etc.)
  • Exclude: AST fuzzers
  • Do not test
  • Woolen Wolfdog
  • Upload binaries for special builds
  • Disable merge-commit
  • Disable CI cache

@arthurpassos
Copy link
Contributor Author

arthurpassos commented Oct 25, 2024

@copperybean any insights would be appreciated

@nickitat nickitat added the can be tested Allows running workflows for external contributors label Oct 29, 2024
@robot-ch-test-poll4 robot-ch-test-poll4 added the pr-improvement Pull request with some product improvements label Oct 29, 2024
@robot-ch-test-poll
Copy link
Contributor

robot-ch-test-poll commented Oct 29, 2024 •
edited
Loading

This is an automated comment for commit 33fdddf with description of existing statuses. It's updated for the latest CI running

❌ Click here to open a full report in a separate page

Check nameDescriptionStatus
Stateless testsRuns stateless functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc❌ failure
Successful checks
Check nameDescriptionStatus
AST fuzzerRuns randomly generated queries to catch program errors. The build type is optionally given in parenthesis. If it fails, ask a maintainer for help✅ success
BuildsThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
ClickBenchRuns [ClickBench](https://github.com/ClickHouse/ClickBench/) with instant-attach table✅ success
Compatibility checkChecks that clickhouse binary runs on distributions with old libc versions. If it fails, ask a maintainer for help✅ success
Docker keeper imageThe check to build and optionally push the mentioned image to docker hub✅ success
Docker server imageThe check to build and optionally push the mentioned image to docker hub✅ success
Docs checkBuilds and tests the documentation✅ success
Fast testNormally this is the first check that is ran for a PR. It builds ClickHouse and runs most of stateless functional tests, omitting some. If it fails, further checks are not started until it is fixed. Look at the report to see which tests fail, then reproduce the failure locally as described here✅ success
Flaky testsChecks if new added or modified tests are flaky by running them repeatedly, in parallel, with more randomization. Functional tests are run 100 times with address sanitizer, and additional randomization of thread scheduling. Integration tests are run up to 10 times. If at least once a new test has failed, or was too long, this check will be red. We don't allow flaky tests, read the doc✅ success
Install packagesChecks that the built packages are installable in a clear environment✅ success
Integration testsThe integration tests report. In parenthesis the package type is given, and in square brackets are the optional part/total tests✅ success
Performance ComparisonMeasure changes in query performance. The performance test report is described in detail here. In square brackets are the optional part/total tests✅ success
Stateful testsRuns stateful functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc✅ success
Stress testRuns stateless functional tests concurrently from several clients to detect concurrency-related errors✅ success
Style checkRuns a set of checks to keep the code style clean. If some of tests failed, see the related log from the report✅ success
Unit testsRuns the unit tests for different release types✅ success
Upgrade checkRuns stress tests on server version from last release and then tries to upgrade it to the version from the PR. It checks if the new server can successfully startup without any errors, crashes or sanitizer asserts✅ success

@arthurpassos arthurpassos marked this pull request as ready for review October 29, 2024 12:38
@arthurpassos arthurpassos mentioned this pull request Oct 29, 2024
Closed
vdimir
vdimir reviewed Oct 29, 2024
View reviewed changes
tests/queries/0_stateless/03254_parquet_bool_native_reader.sh
Comment on lines +8 to +14
USER_FILES_PATH=$($CLICKHOUSE_CLIENT_BINARY --query "select _path,_file from file('nonexist.txt', 'CSV', 'val1 char')" 2>&1 | grep Exception | awk '{gsub("/nonexist.txt","",$9); print $9}')

WORKING_DIR="${USER_FILES_PATH}/${CLICKHOUSE_TEST_UNIQUE_NAME}"

mkdir -p "${WORKING_DIR}"

DATA_FILE="${CUR_DIR}/data_parquet/nullbool.parquet"
Copy link
Member

@vdimir vdimir Oct 29, 2024

Choose a reason for hiding this comment

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

I'd just do INSERT INTO TABLE FUNCTION file('nullbool.parquet') SELECT base64Decode('....') embedding data into test (since file is pretty small) and then work with file('nullbool.parquet'). It can be done even in .sql test

@vdimir
Copy link
Member

vdimir commented Oct 30, 2024

Stateless tests (msan) [4/4] — Server died, fail: 9, passed: 1585, skipped: 25 Details

Sanitizer report is relevant:

https://s3.amazonaws.com/clickhouse-test-reports/71055/573204c3033a21f5dad745e946e102a596d26e6e/stateless_tests__msan__[4_4]/stderr.log

0 0x560d9c95b212 in malloc (/usr/bin/clickhouse+0x8489212) (BuildId: bc254b4c582e73d832390108a0b992795df0e627)
    #1 0x560db3101c56 in void* (anonymous namespace)::allocNoTrack<false, false>(unsigned long, unsigned long) build_docker/./src/Common/Allocator.cpp:94:19
    #2 0x560db3101c56 in Allocator<false, false>::alloc(unsigned long, unsigned long) build_docker/./src/Common/Allocator.cpp:150:18
    #3 0x560dcc9659db in void DB::PODArrayBase<1ul, 4096ul, Allocator<false, false>, 63ul, 64ul>::alloc<>(unsigned long) build_docker/./src/Common/PODArray.h:123:65
    #4 0x560dcc9659db in void DB::PODArrayBase<1ul, 4096ul, Allocator<false, false>, 63ul, 64ul>::realloc<>(unsigned long) build_docker/./src/Common/PODArray.h:148:13
    #5 0x560dcc9659db in void DB::PODArrayBase<1ul, 4096ul, Allocator<false, false>, 63ul, 64ul>::reserve_exact<>(unsigned long) build_docker/./src/Common/PODArray.h:238:13
    #6 0x560dcc9659db in DB::ColumnVector<char8_t>::reserve(unsigned long) build_docker/./src/Columns/ColumnVector.h:180:14
    #7 0x560dd14cf616 in DB::ParquetLeafColReader<DB::ColumnVector<char8_t>>::resetColumn(unsigned long) build_docker/./src/Processors/Formats/Impl/Parquet/ParquetLeafColReader.cpp:283:13

vdimir
vdimir requested changes Oct 30, 2024
View reviewed changes
Copy link
Member

@vdimir vdimir left a comment

Choose a reason for hiding this comment

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

@arthurpassos, could you please check sanitizer report?

@arthurpassos
Copy link
Contributor Author

arthurpassos commented Oct 30, 2024 •
edited
Loading

@arthurpassos, could you please check sanitizer report?

For sure. Is there a way we can get the offending query? I am skimming through the log files, but no luck yet

I thought it was fuzzer, but it is msan. Will investigate

vdimir
vdimir approved these changes Oct 31, 2024
View reviewed changes
Copy link
Member

@vdimir vdimir left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

@vdimir vdimir added this pull request to the merge queue Oct 31, 2024
@arthurpassos
Copy link
Contributor Author

arthurpassos commented Oct 31, 2024

Thanks for your contribution!

Thank you for looking into it :D

Merged via the queue into ClickHouse:master with commit a6c6ff5 Oct 31, 2024
@robot-ch-test-poll3 robot-ch-test-poll3 added the pr-synced-to-cloud The PR is synced to the cloud repo label Oct 31, 2024
arthurpassos pushed a commit to Altinity/ClickHouse that referenced this pull request Jan 9, 2025
@vdimir @arthurpassos
Merge pull request ClickHouse#71055 from arthurpassos/support_for_boo…
e991a7f 
…lean_parquet_native_reader

Boolean support for parquet native reader
@svb-alt svb-alt mentioned this pull request May 23, 2025
Closed
31 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vdimir vdimir vdimir approved these changes

Assignees

@vdimir vdimir

Labels

can be tested Allows running workflows for external contributors pr-improvement Pull request with some product improvements pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@arthurpassos @robot-ch-test-poll @vdimir @nickitat @robot-ch-test-poll4 @robot-ch-test-poll3