Skip to content
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

Remove database prefixes from ClickHouse table creation scripts #83

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

iuwqyir
Copy link
Collaborator

@iuwqyir iuwqyir commented Oct 1, 2024

TL;DR

Removed database prefixes from table names in ClickHouse SQL scripts and updated engine type for logs table.

What changed?

  • Removed database prefixes (orchestrator., base., and staging.) from all table names in ClickHouse SQL creation scripts.
  • Changed the engine for the logs table from SharedReplacingMergeTree to ReplacingMergeTree. Shared is only available in hosted version and will be applied automatically if you create a ReplacingMergeTree

How to test?

  1. Execute the updated SQL scripts in a ClickHouse environment.
  2. Verify that tables are created without database prefixes.
  3. Confirm that the logs table is created with the ReplacingMergeTree engine.
  4. Ensure that all other table structures and settings remain unchanged.

Why make this change?

  • Removing database prefixes allows for more flexibility in table placement and database organization. We don't want to enforce if these tables are used in one database or several

@iuwqyir iuwqyir changed the title remove database from scripts Remove database prefixes from ClickHouse table creation scripts Oct 1, 2024
Copy link
Collaborator Author

iuwqyir commented Oct 1, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @iuwqyir and the rest of your teammates on Graphite Graphite

@iuwqyir iuwqyir requested a review from AmineAfia October 1, 2024 21:53
@iuwqyir iuwqyir marked this pull request as ready for review October 1, 2024 21:53
Copy link
Collaborator

@AmineAfia AmineAfia left a comment

Choose a reason for hiding this comment

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

I though we are putting the orchestrator and staging in the same db as main. but still isolate the chains

@iuwqyir
Copy link
Collaborator Author

iuwqyir commented Oct 1, 2024

I though we are putting the orchestrator and staging in the same db as main. but still isolate the chains

We can, but is there a need? Also since this is currently run manually anyways, anyone can decide the database division they want by changing the database

@iuwqyir iuwqyir requested a review from AmineAfia October 2, 2024 11:35
Copy link
Collaborator

@AmineAfia AmineAfia left a comment

Choose a reason for hiding this comment

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

my question was about the amount of data we are putting in one table, but we talked about that with jake yesterday

@iuwqyir iuwqyir merged commit 61d5b68 into main Oct 3, 2024
4 checks passed
@iuwqyir iuwqyir deleted the 10-02-remove_database_from_scripts branch October 3, 2024 14:39
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