Skip to content

feat(clickhouse-driver): add intelligent DDL handling for unsupported operations #9845

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Greenystuff
Copy link

@Greenystuff Greenystuff commented Aug 3, 2025

Check List

  • Tests have been run in packages where changes made if available
  • Linter has been run for changed code
  • Tests for the changes have been added if not covered yet
  • Docs have been added / updated if required

Issue Reference this PR resolves

No specific issue reference - this is a proactive enhancement to improve ClickHouse driver compatibility

Description of Changes Made

This PR adds intelligent DDL (Data Definition Language) handling to the ClickHouse driver to handle operations that are not natively supported by certain ClickHouse storage engines, particularly the Log engine.

Key Changes:

  • Enhanced DDL detection with comprehensive keyword support (CREATE, ALTER, DROP, TRUNCATE, RENAME, ATTACH, DETACH, GRANT, REVOKE)
  • Implemented storage engine compatibility layer for ALTER TABLE ADD COLUMN on Log engine
  • Added specific ClickHouse error code handling with detailed error messages
  • Comprehensive test coverage for DDL operations and mixed DDL/DML scenarios

Technical Implementation:

  • DDL statements now use connection.command() instead of connection.query() to avoid JSON parsing errors
  • For ALTER TABLE ADD COLUMN on Log engine: automatically recreates table with new schema, copies data, drops old table, renames new table
  • Enhanced error messages include ClickHouse error codes and query context for better debugging
  • Backward-compatible enhancement with no breaking changes

Testing:

  • All existing tests pass
  • New integration tests for DDL handling pass
  • Mixed DDL/DML operations work correctly
  • ALTER TABLE ADD COLUMN compatibility layer works as expected

Problem Solved:
The ClickHouse Log engine doesn't support ALTER TABLE ADD COLUMN operations, returning error code 48 (NOT_IMPLEMENTED). This PR implements a compatibility layer that automatically handles this limitation by recreating the table with the new schema, ensuring data integrity while maintaining the expected behavior.

@Greenystuff Greenystuff requested a review from a team as a code owner August 3, 2025 13:05
@github-actions github-actions bot added the pr:community Contribution from Cube.js community members. label Aug 3, 2025
@Greenystuff
Copy link
Author

Greenystuff commented Aug 3, 2025

Hi there! 👋

I'm a FullStack developer with 10+ years of experience, currently working for a company that specializes in Business Intelligence using Cube.js and ClickHouse.

This PR addresses a real-world issue we encountered in production with Cube's pre-aggregation feature. We were getting this error:

Error: Query failed: SyntaxError: Unexpected end of JSON input; query id: 53b7c702-e773-496a-b5c0-a6f21b5377a5

The problem occurs when Cube tries to execute DDL operations (like CREATE TABLE or ALTER TABLE) through the ClickHouse driver. ClickHouse itself doesn't support using .query() for DDL statements - it requires .command() instead, and the driver was incorrectly trying to parse the response as JSON even though DDL statements don't return data.
Sources :
https://clickhouse.com/docs/integrations/javascript#query-method
https://clickhouse.com/docs/integrations/javascript#command-method

What this PR does:

  • Intelligently detects DDL operations and uses .command() instead of .query() as required by ClickHouse
  • Provides better error messages for ClickHouse-specific issues
  • Implements a compatibility layer for storage engines that don't support certain DDL operations
  • Maintains full backward compatibility

Why I'm contributing:
I believe in giving back to the open-source community, especially when we can solve real problems that others might face. This enhancement should help anyone using Cube.js with ClickHouse, particularly in production environments.

Testing:
Our unit tests confirmed that the current driver cannot handle DDL operations with .query() - ClickHouse simply doesn't support it. This PR fixes that fundamental incompatibility.

I'm open to any discussion about this implementation and happy to iterate based on feedback. Feel free to ask questions or suggest improvements!

P.S. This is my first PR ever, so please be gentle! But seriously, don't hesitate to question anything - I'm here to learn and improve. Looking forward to contributing more to this amazing project!

@Greenystuff Greenystuff marked this pull request as draft August 4, 2025 10:20
@Greenystuff Greenystuff marked this pull request as ready for review August 4, 2025 13:04
@Greenystuff
Copy link
Author

Greenystuff commented Aug 4, 2025

I had to build it with a custom docker image to test It before asking review and I can tell you that okay now. With this fix there is no longer parsing error
I'm using this custom docker image and I can store pre-aggregates in my source database on Click House.
I have to let the dev mode true if I want that internals aggregation works. I don't know if I'm wrong with the usage but It works perfectly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:community Contribution from Cube.js community members.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant