Fix/json reader fixedstring v2 #448
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rsions Problem: ClickHouse returns FixedString as plain text in JSON format, but the connector was trying to decode it as Base64, causing InvalidFormatException. Solution: Use pattern matching with guard to check if the JSON node is textual. - If textual (FixedString): decode as UTF-8 bytes - If not textual (true binary): decode as Base64 Applied to Spark 3.3, 3.4, and 3.5.
|
Shimon Steinitz seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
![windsurf-bot[bot]](https://avatars.githubusercontent.com/in/1066231?s=60&v=4){kind=small}
Comment on lines
+48
to
+49
| env: | ||
| CLICKHOUSE_PASSWORD: ${{ secrets.INTEGRATIONS_TEAM_TESTS_CLOUD_PASSWORD_SMT }} |
There was a problem hiding this comment.
There's an inconsistency in environment variable naming. The job defines CLICKHOUSE_CLOUD_PASSWORD but the wake-up script uses CLICKHOUSE_PASSWORD. Consider using the same variable name in both places or explicitly mapping between them.
mzitnik
approved these changes
Nov 6, 2025
BentsiLeviav
added a commit
that referenced
this pull request
Nov 16, 2025
* Upgrade Java Client to V2 syncQuery & syncInsert * Refactor to use the new client v2 api * Add timeout to query operation * Clean NodeClient * Change binary reader * Update client version * Fix project to use snapshots * merge with main * run spotlessScalaApply and implement readAllBytes since java 8 does not support * Remove unneeded remarks * Chanage to client version 0.9.3 * Update socket timeout in new client * Change max connections to 20 * ConnectTimeout to 1200000 * Add 3 sec to sleep * Setting a new setConnectionRequestTimeout for experiment * spotlessScalaApply fix * Fix/json reader fixedstring v2 (#448) * Wake up ClickHouse Cloud instance before tests (#429) * fix: Handle FixedString as plain text in JSON reader for all Spark versions Problem: ClickHouse returns FixedString as plain text in JSON format, but the connector was trying to decode it as Base64, causing InvalidFormatException. Solution: Use pattern matching with guard to check if the JSON node is textual. - If textual (FixedString): decode as UTF-8 bytes - If not textual (true binary): decode as Base64 Applied to Spark 3.3, 3.4, and 3.5. --------- Co-authored-by: Bentsi Leviav <bentsi.leviav@clickhouse.com> Co-authored-by: Shimon Steinitz <shimonsteinitz@Shimons-MacBook-Pro.local> * Added reader and writer tests (#449) * Wake up ClickHouse Cloud instance before tests (#429) * feat: Add comprehensive read test coverage for Spark 3.3, 3.4, and 3.5 Add shared test trait ClickHouseReaderTestBase with 48 test scenarios covering: - All primitive types (Boolean, Byte, Short, Int, Long, Float, Double) - Large integers (UInt64, Int128, UInt128, Int256, UInt256) - Decimals (Decimal32, Decimal64, Decimal128) - Date/Time types (Date, Date32, DateTime, DateTime32, DateTime64) - String types (String, UUID, FixedString) - Enums (Enum8, Enum16) - IP addresses (IPv4, IPv6) - JSON data - Collections (Arrays, Maps) - Edge cases (empty strings, long strings, empty arrays, nullable variants) Test suites for Binary and JSON read formats. Test results: 96 tests per Spark version (288 total) - Binary format: 47/48 passing - JSON format: 47/48 passing - Overall: 94/96 passing per version (98% pass rate) Remaining failures are known bugs with fixes on separate branches. * feat: Add comprehensive write test coverage for Spark 3.3, 3.4, and 3.5 Add shared test trait ClickHouseWriterTestBase with 17 test scenarios covering: - Primitive types (Boolean, Byte, Short, Int, Long, Float, Double) - Decimal types - String types (regular and empty strings) - Date and Timestamp types - Collections (Arrays and Maps, including empty variants) - Nullable variants Test suites for JSON and Arrow write formats. Note: Binary write format is not supported (only JSON and Arrow). Test results: 34 tests per Spark version (102 total) - JSON format: 17/17 passing (100%) - Arrow format: 17/17 passing (100%) - Overall: 34/34 passing per version (100% pass rate) Known behavior: Boolean values write as BooleanType but read back as ShortType (0/1) due to ClickHouse storing Boolean as UInt8. * style: Apply spotless formatting * style: Apply spotless formatting for Spark 3.3 and 3.4 Remove trailing whitespace from test files to pass CI spotless checks. * fix: Change write format from binary to arrow in BinaryReaderSuite The 'binary' write format option doesn't exist. Changed to 'arrow' which is a valid write format option. Applied to Spark 3.3, 3.4, and 3.5. * test: Add nullable tests for ShortType, IntegerType, and LongType Added missing nullable variant tests to ensure comprehensive coverage: - decode ShortType - nullable with null values (Nullable(Int16)) - decode IntegerType - nullable with null values (Nullable(Int32)) - decode LongType - nullable with null values (Nullable(Int64)) These tests verify that nullable primitive types correctly handle NULL values in both Binary and JSON read formats. Applied to Spark 3.3, 3.4, and 3.5. Total tests per Spark version: 51 (was 48) Total across all versions: 153 (was 144) * Refactor ClickHouseReaderTestBase: Add nullable tests and organize alphabetically - Add missing nullable test cases for: Date32, Decimal32, Decimal128, UInt16, UUID, DateTime64 - Organize all 69 tests alphabetically by data type for better maintainability - Ensure comprehensive coverage with both nullable and non-nullable variants for all data types - Apply changes consistently across Spark 3.3, 3.4, and 3.5 * ci: Skip cloud tests on forks where secrets are unavailable Add repository check to cloud workflow to prevent failures on forks that don't have access to ClickHouse Cloud secrets. Tests will still run on the main repository where secrets are properly configured. * Refactor and enhance Reader/Writer tests for all Spark versions - Add BooleanType tests to Reader (2 tests) with format-aware assertions - Add 6 new tests to Writer: nested arrays, arrays with nullable elements, multiple Decimal precisions (18,4 and 38,10), Map with nullable values, and StructType - Reorder all tests lexicographically for better organization - Writer tests increased from 17 to 33 tests - Reader tests increased from 69 to 71 tests - Remove section header comments for cleaner code - Apply changes to all Spark versions: 3.3, 3.4, and 3.5 - All tests now properly sorted alphabetically by data type and variant * style: Apply spotless formatting to Reader/Writer tests --------- Co-authored-by: Bentsi Leviav <bentsi.leviav@clickhouse.com> Co-authored-by: Shimon Steinitz <shimon.steinitz@clickhouse.com> * Fix BinaryReader to handle new Java client types - Fix DecimalType: Handle both BigInteger (Int256/UInt256) and BigDecimal (Decimal types) - Fix ArrayType: Direct call to BinaryStreamReader.ArrayValue.getArrayOfObjects() - Fix StringType: Handle UUID, InetAddress, and EnumValue types - Fix DateType: Handle both LocalDate and ZonedDateTime - Fix MapType: Handle all util.Map implementations Removed reflection and defensive pattern matching for better performance. All 34 Binary Reader test failures are now fixed (71/71 tests passing). Fixes compatibility with new Java client API in update-java-client-version branch. * Add high-precision decimal tests with tolerance - Add Decimal(18,4) test with 0.001 tolerance for JSON/Arrow formats - Documents precision limitation for decimals with >15-17 significant digits - Uses tolerance-based assertions to account for observed precision loss - Binary format preserves full precision (already tested in Binary Reader suite) - All 278 tests passing * Simplify build-and-test workflow trigger to run on all pushes * Fix Scala 2.13 compatibility for nested arrays - Convert mutable.ArraySeq to Array in ClickHouseJsonReader to ensure immutable collections - Add test workaround for Spark's Row.getSeq behavior in Scala 2.13 - Fix Spotless formatting: remove trailing whitespace in ClickHouseBinaryReader - Applied to all Spark versions: 3.3, 3.4, 3.5 * Update java version to 0.9.4 * Enable compression * add logging TPCDSClusterSuite & change client buffers * Change InputStream read code * Remove hard coded settings for experiments * Clean log from insert method --------- Co-authored-by: Shimon Steinitz <shimonste@gmail.com> Co-authored-by: Bentsi Leviav <bentsi.leviav@clickhouse.com> Co-authored-by: Shimon Steinitz <shimonsteinitz@Shimons-MacBook-Pro.local> Co-authored-by: Shimon Steinitz <shimon.steinitz@clickhouse.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix FixedString handling in JSON reader
Problem
ClickHouse returns
FixedStringcolumns as plain text in JSON format, but the connector was attempting to decode them as Base64-encoded binary data. This causedInvalidFormatExceptionwhen reading FixedString columns with JSON format.Root Cause
The
ClickHouseJsonReadertreated allBinaryTypecolumns the same way, callingjsonNode.binaryValuewhich expects Base64-encoded data. However, ClickHouse's JSON format returns:"hello")"aGVsbG8=")Solution
Added pattern matching with guard to distinguish between textual and binary JSON nodes:
Changes
Applied to all Spark versions:
spark-3.3/clickhouse-spark/src/main/scala/com/clickhouse/spark/read/format/ClickHouseJsonReader.scalaspark-3.4/clickhouse-spark/src/main/scala/com/clickhouse/spark/read/format/ClickHouseJsonReader.scalaspark-3.5/clickhouse-spark/src/main/scala/com/clickhouse/spark/read/format/ClickHouseJsonReader.scalaTesting
The fix is validated by comprehensive test coverage in PR #[test-coverage-pr]:
ClickHouseReaderTestBaseincludes FixedString testsdecode BinaryType - FixedString