Skip to content

Conversation

shehabgamin
Copy link
Contributor

@shehabgamin shehabgamin commented Aug 26, 2025

Updated to Sail 0.3.3 and also added sail-partitioned.

Didn't update any results, wasn't sure if I'm supposed to do that or not.

@CLAassistant
Copy link

CLAassistant commented Aug 26, 2025

CLA assistant check
All committers have signed the CLA.

@shehabgamin shehabgamin marked this pull request as ready for review August 26, 2025 03:56
Copy link
Member

Choose a reason for hiding this comment

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

The files in sail-partitioned/ mostly duplicate the files in sail/.
May I ask you to create symbolic links where possible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have an example to share?

Looking at clickhouse-parquet-partitioned, it's also a duplicate of the files in clickhouse-parquet:

Copy link
Member

Choose a reason for hiding this comment

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

@rschu1ze, this does not matter, and we didn't use symlinks before. Copy-paste is ok.

export TZ=Etc/UTC
sudo ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone

echo "Install Rust"
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice if we could either use the Rust packages that ship with Ubuntu 24.10 (if not too old), or install at least in a local directory (not the user home directory).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rschu1ze I just copied what I saw in DataFusion:
https://github.com/ClickHouse/ClickBench/blob/main/datafusion/benchmark.sh#L3-L7

Also, It looks like modifying the user home directory was a recent addition in #496

export HOME=${HOME:=~}

Copy link
Member

Choose a reason for hiding this comment

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

@rschu1ze, this does not matter, ClickBench runs in short-lived VMs.

@shehabgamin shehabgamin requested a review from rschu1ze August 29, 2025 00:53
@shehabgamin
Copy link
Contributor Author

@rschu1ze @alexey-milovidov Just checking in here

@alexey-milovidov alexey-milovidov self-assigned this Aug 30, 2025
@alexey-milovidov alexey-milovidov merged commit 7c3d194 into ClickHouse:main Aug 30, 2025
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.

4 participants