Skip to content

Conversation

@WZhuo
Copy link
Contributor

@WZhuo WZhuo commented Dec 30, 2025

The url encode and decode function will be used for generating partitions' path, so move it to the common util for later use.


list(APPEND
ICEBERG_STATIC_BUILD_INTERFACE_LIBS
cpr::cpr
Copy link
Member

Choose a reason for hiding this comment

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

Can we avoid adding cpr to the core iceberg library? Why is it needed to generate partition path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Result<std::string>  LocationProvider::NewDataLocation(const PartitionSpec& spec,
                                              const PartitionValues& partition_data,
                                              const std::string& filename) = 0;

In LocationProvider::NewDataLocation, we must do url encoding for partition names and partition values.
If not import cpr to iceberg core, maybe we need implement Url encode/decode by ourselves.

Copy link
Member

Choose a reason for hiding this comment

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

Got it. According to prior discussions in the community, we should keep minimal dependencies of libiceberg. If the only reason is to escape partition path, should we implement our own utility function? IIUC, partition name and value can only use a limited set of characters.

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.

2 participants