Re: [PR] Allow PartitionField's field_id is missing in Iceberg v1 [iceberg-cpp]

Smith-Cruise commented on PR #121: URL: https://github.com/apache/iceberg-cpp/pull/121#issuecomment-2984085883 > > I believe @Fokko's idea has already been implemented > > Nice, that's great to hear! @Smith-Cruise Could you fix the CI? :) It just looks like a network issue; I th

Re: [PR] Allow PartitionField's field_id is missing in Iceberg v1 [iceberg-cpp]

Fokko commented on PR #121: URL: https://github.com/apache/iceberg-cpp/pull/121#issuecomment-2982779576 > I believe @Fokko's idea has already been implemented Nice, that's great to hear! @Smith-Cruise Could you fix the CI? :) -- This is an automated message from the Apache Git Servi

Re: [PR] Allow PartitionField's field_id is missing in Iceberg v1 [iceberg-cpp]

Smith-Cruise commented on PR #121: URL: https://github.com/apache/iceberg-cpp/pull/121#issuecomment-2982430729 @wgtmac CI run failed, could you help me to rerun it? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the U

Re: [PR] Allow PartitionField's field_id is missing in Iceberg v1 [iceberg-cpp]

Smith-Cruise commented on PR #121: URL: https://github.com/apache/iceberg-cpp/pull/121#issuecomment-2969574576 > I believe @Fokko's idea has already been implemented as in https://github.com/apache/iceberg-cpp/blob/main/src/iceberg/json_internal.cc#L1058-L1061. Actually my intention is that

Re: [PR] Allow PartitionField's field_id is missing in Iceberg v1 [iceberg-cpp]

wgtmac commented on PR #121: URL: https://github.com/apache/iceberg-cpp/pull/121#issuecomment-2969245961 I believe @Fokko's idea has already been implemented as in https://github.com/apache/iceberg-cpp/blob/main/src/iceberg/json_internal.cc#L1058-L1061. Actually my intention is that `json_i

Re: [PR] Allow PartitionField's field_id is missing in Iceberg v1 [iceberg-cpp]

Smith-Cruise commented on PR #121: URL: https://github.com/apache/iceberg-cpp/pull/121#issuecomment-2961865856 > Generally I think the logic with `PartitionFieldFromJson(json, /*allow_field_id_missing=*/true)` and current logic is equal, but this LGTM. Also cc @wgtmac Just ensure tha

Re: [PR] Allow PartitionField's field_id is missing in Iceberg v1 [iceberg-cpp]

Smith-Cruise commented on code in PR #121: URL: https://github.com/apache/iceberg-cpp/pull/121#discussion_r2139625779 ## src/iceberg/json_internal.cc: ## @@ -1053,14 +1052,10 @@ Status ParsePartitionSpecs(const nlohmann::json& json, int8_t format_version, int32_t next_part

Re: [PR] Allow PartitionField's field_id is missing in Iceberg v1 [iceberg-cpp]

mapleFU commented on code in PR #121: URL: https://github.com/apache/iceberg-cpp/pull/121#discussion_r2139352798 ## src/iceberg/json_internal.cc: ## @@ -1053,14 +1052,10 @@ Status ParsePartitionSpecs(const nlohmann::json& json, int8_t format_version, int32_t next_partition

Re: [PR] Allow PartitionField's field_id is missing in Iceberg v1 [iceberg-cpp]

Smith-Cruise commented on PR #121: URL: https://github.com/apache/iceberg-cpp/pull/121#issuecomment-2958810236 > https://github.com/apache/iceberg/blob/7b8bd29ce80bc78ed882f0613f7570e78e325988/core/src/main/java/org/apache/iceberg/PartitionSpecParser.java#L137-L144 Your idea is correc

Re: [PR] Allow PartitionField's field_id is missing in Iceberg v1 [iceberg-cpp]

Fokko commented on PR #121: URL: https://github.com/apache/iceberg-cpp/pull/121#issuecomment-2958639839 I think we should improve this in another way. I don't think we should set it to `-1` when it is missing: https://github.com/apache/iceberg-cpp/blob/ed49d1e8f0dc31d99d4fff437a6c158

Re: [PR] Allow PartitionField's field_id is missing in Iceberg v1 [iceberg-cpp]

mapleFU commented on code in PR #121: URL: https://github.com/apache/iceberg-cpp/pull/121#discussion_r2137433518 ## src/iceberg/json_internal.cc: ## @@ -1053,7 +1053,7 @@ Status ParsePartitionSpecs(const nlohmann::json& json, int8_t format_version, int32_t next_partition_f