Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-21 Thread via GitHub
lidavidm commented on PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#issuecomment-2606393374 Ok, hopefully that fixes macOS. It appears C++20 support on the version of clang shipped by macOS is still flaky: https://godbolt.org/z/W9rTexc13 Maybe we want to avoid C++2

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-20 Thread via GitHub
lidavidm commented on PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#issuecomment-2603698520 I've implemented the Type subclasses. I've also added a trait for ToString/std::format formatting (might as well use the new C++ features if we're requiring C++20). Although it appears c

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-20 Thread via GitHub
lidavidm commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1923025444 ## src/iceberg/type.h: ## @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOT

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-20 Thread via GitHub
lidavidm commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1922923718 ## src/iceberg/schema.h: ## @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NO

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-20 Thread via GitHub
gaborkaszab commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1922625409 ## src/iceberg/type.h: ## @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-20 Thread via GitHub
gaborkaszab commented on PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#issuecomment-2602857029 > Just to make sure @gaborkaszab @wgtmac: are we ok with the Arrow-style type representation here? (Types are represented by a class hierarchy, erased behind smart pointers; nested ty

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-20 Thread via GitHub
mapleFU commented on PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#issuecomment-2602610638 LGTM -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe,

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-20 Thread via GitHub
wgtmac commented on PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#issuecomment-2602518959 I vote +1 for the Arrow-style. I haven't checked the `cuDF` style in detail but it looks more like the C-style. The extra benefit of Arrow-style is that it might be simpler to implement th

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-20 Thread via GitHub
lidavidm commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1922131399 ## src/iceberg/type.h: ## @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOT

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-20 Thread via GitHub
mapleFU commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1921978447 ## src/iceberg/type.h: ## @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTI

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-19 Thread via GitHub
lidavidm commented on PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#issuecomment-2601203055 Arrow-style does let you do a bunch of compile-time metaprogramming (e.g. see arrow::TypeTraits) -- This is an automated message from the Apache Git Service. To respond to the message,

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-19 Thread via GitHub
lidavidm commented on PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#issuecomment-2601202575 Just to make sure @gaborkaszab @wgtmac: are we ok with the Arrow-style type representation here? (Types are represented by a class hierarchy, erased behind smart pointers; nested types s

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-17 Thread via GitHub
wgtmac commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1920212826 ## src/iceberg/schema.h: ## @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTI

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-17 Thread via GitHub
lidavidm commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1920061333 ## src/iceberg/schema.h: ## @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NO

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-17 Thread via GitHub
lidavidm commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1920059796 ## src/iceberg/type.h: ## @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOT

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-17 Thread via GitHub
gaborkaszab commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1919986863 ## src/iceberg/schema.h: ## @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-17 Thread via GitHub
lidavidm commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1919988333 ## src/iceberg/type.h: ## @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOT

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-17 Thread via GitHub
lidavidm commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1919984826 ## src/iceberg/type.h: ## @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOT

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-17 Thread via GitHub
lidavidm commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1919982972 ## src/iceberg/schema.h: ## @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NO

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-17 Thread via GitHub
gaborkaszab commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1919778107 ## src/iceberg/schema.h: ## @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-17 Thread via GitHub
lidavidm commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1919981922 ## src/iceberg/schema.h: ## @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NO

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-17 Thread via GitHub
lidavidm commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1919981227 ## src/iceberg/schema.h: ## @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NO

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-17 Thread via GitHub
gaborkaszab commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1919778107 ## src/iceberg/schema.h: ## @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-16 Thread via GitHub
lidavidm commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1919659067 ## src/iceberg/type.h: ## @@ -0,0 +1,120 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOT

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-16 Thread via GitHub
lidavidm commented on PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#issuecomment-2597595950 I added Primitive/NestedType; I'll start filling out definitions and the rest of the types next, along with adding the visitors to iceberg-arrow. -- This is an automated message from t

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-16 Thread via GitHub
lidavidm commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1919669923 ## src/iceberg/type.h: ## @@ -0,0 +1,120 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOT

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-16 Thread via GitHub
lidavidm commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1919662198 ## src/iceberg/schema.h: ## @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NO

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-16 Thread via GitHub
lidavidm commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1919661788 ## src/iceberg/type.h: ## @@ -0,0 +1,120 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOT

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-16 Thread via GitHub
wgtmac commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1919463629 ## src/iceberg/type_fwd.h: ## @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NO

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-16 Thread via GitHub
Fokko commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1918197785 ## src/iceberg/type_fwd.h: ## @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOT

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-15 Thread via GitHub
lidavidm commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1917538165 ## src/iceberg/schema.h: ## @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NO

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-15 Thread via GitHub
lidavidm commented on code in PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#discussion_r1917537751 ## src/iceberg/type_fwd.h: ## @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-15 Thread via GitHub
lidavidm commented on PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#issuecomment-2594268386 For here I've chosen something somewhat resembling Arrow C++. However I think Schema and Field (unlike Arrow) should not always be wrapped in smart pointers. Type is still in a smart poi

Re: [PR] WIP: Add headers for type/field/schema [iceberg-cpp]

2025-01-15 Thread via GitHub
lidavidm commented on PR #31: URL: https://github.com/apache/iceberg-cpp/pull/31#issuecomment-2594267414 Note: we could simplify the data type definition greatly if we adopt something like Arrow Java/[cuDF](https://docs.rapids.ai/api/libcudf/stable/classcudf_1_1data__type). Type would be a