Skip to content

Conversation

@kakagri
Copy link
Contributor

@kakagri kakagri commented Oct 17, 2025

fixing issue with hosted clickhouse databases + naming issue, this is 100% vibe coded so be careful when merging :)

@vercel
Copy link

vercel bot commented Oct 17, 2025

@kakagri is attempting to deploy a commit to the joshaavecom's projects Team on Vercel.

A member of the Team first needs to authorize it.

let mut sql = String::new();

sql.push_str("DROP TABLE IF EXISTS rindexer_internal.latest_block;");
sql.push_str(&format!("DROP TABLE IF EXISTS {}.rindexer_internal_latest_block;", database_name));
Copy link
Contributor

Choose a reason for hiding this comment

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

To clarify this is because in hosted environemnt you want to just have default as the database and have tables be on the main db right

[package]
name = "rindexer_cli"
version = "0.28.0"
version = "0.28.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure we should bump this.

### Bug fixes
-------------------------------------------------
- fix: add tls for hosted clickhouse databases
- fix: ClickHouse now uses single database from `CLICKHOUSE_DB` env var instead of requiring `CREATE DATABASE` privileges, rationale: giving all permissions to one script to drop any database is unreasonable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add these changes under the ## Changes Not Deployed section rather than making a new release.

This allows multiple pending changes to be deployed with a new version

### Bug fixes
-------------------------------------------------
- fix: add tls for hosted clickhouse databases
- fix: ClickHouse now uses single database from `CLICKHOUSE_DB` env var instead of requiring `CREATE DATABASE` privileges, rationale: giving all permissions to one script to drop any database is unreasonable.
Copy link
Contributor

@jaggad jaggad Oct 20, 2025

Choose a reason for hiding this comment

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

ClickHouse now uses single database from CLICKHOUSE_DB env var instead of requiring CREATE DATABASE privileges, rationale: giving all permissions to one script to drop any database is unreasonable.

Rationale makes a lot of sense to me, is a reasonable DBA perspective for permissions management. But this can be achieved by granting more granular permissions.

[package]
name = "rindexer"
version = "0.28.0"
version = "0.28.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't bump here

Suggested change
version = "0.28.1"
version = "0.28.0"

last_synced_block: u64,
}

let database_name = clickhouse.get_database_name();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just call clickhouse.database_name instead of the getter fn

Copy link
Contributor

@jaggad jaggad left a comment

Choose a reason for hiding this comment

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

Questions

@kakagri is the main purpose for this change specifically to create a user in a hosted environment that doens't need permissions granted to create and drop databases?

From what I understand there's no limitations in Clickhouse Cloud to create other databases (which act like Postgres schema's imo).

If you can help clarify the exact problem here I can maybe help, I've spent a while on reviewing this but more context on why this PR change is needed would be nice.

If tighter permissions are the goal here

CREATE DATABASE rindexer_internal;
CREATE DATABASE rindexer_internal_cant_drop;

CREATE TABLE rindexer_internal.test (id UUID) ENGINE MergeTree ORDER BY id;
CREATE TABLE rindexer_internal_cant_drop.test_cant_drop (id UUID) ENGINE MergeTree ORDER BY id;

CREATE USER jaggad IDENTIFIED WITH sha256_password BY 'password';

GRANT DROP DATABASE ON rindexer_internal.* TO jaggad;
GRANT DROP TABLE ON rindexer_internal.* TO jaggad;

Then try connected as the new user

DROP DATABASE rindexer_internal; -- drops ✅ 
DROP DATABASE rindexer_internal_cant_drop; -- doesn't drop, as expected ✅ 

Feedback

Test via examples to see remaining issues.

UniswapV3FactoryPoolCreatedPool::PoolCreated Event processing failed - id: bu5dd9OOvh - topic_id: 0x783cca1c0412dd0d695e784568c96da2e9c22ff989357a2e8b1d9b2b4e6b7118. Retrying... (attempt 8). Error: ClickhouseError: bad response: Code: 81. DB::Exception: Database rindexer_factory_contract_uniswap_v3_factory_pool_created_pool does not exist. (UNKNOWN_DATABASE) (version 25.7.3.13 (official build))
20 October - 11:51:26.601413 ERROR RocketPool::Transfer Event processing failed - id: OEzeDrgHOk - topic_id: 0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef. Retrying... (attempt 3). Error: ClickhouseError: bad response: Code: 81. DB::Exception: Database clickhouse_indexer_rocket_pool does not exist. (UNKNOWN_DATABASE) (version 25.7.3.13 (official build))

@kakagri I think there may be a missed change somewhere in the creation for factory contracts.

Go to examples/clickhouse_factory_indexing and examples/rust_clickhouse and cargo run to see the errors printed in the console for testing.


Remaining unchanged locations

You'll still find lots of references to this in the codebase. E.g see the below insert_bulk used in some locations related to factory indexing.

.insert_bulk(
                &format!("rindexer_internal.{table_name}"),

Also checkout core/src/generator/events_bindings.rs and other files in the codegen section which are fired when rindexer codegen indexer is used and codegens like the following but you've changed it from this layout clickhouse_indexer_rocket_pool.transfer to this layout clickhouse_indexer_rocket_pool_transfer.

let result = context
                .database
                .insert_bulk("clickhouse_indexer_rocket_pool.transfer", &rows, &postgres_bulk_data)
                .await;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants