wgtmac commented on code in PR #757:
URL: https://github.com/apache/iceberg-cpp/pull/757#discussion_r3564516735
##########
src/iceberg/transform.cc:
##########
@@ -234,6 +233,19 @@ bool Transform::CanTransform(const Type& source_type)
const {
std::unreachable();
}
+Status Transform::Validate(const std::shared_ptr<Type>& source_type) const {
+ if (!source_type) {
+ return InvalidArgument("Source type cannot be null for transform {}",
ToString());
Review Comment:
Why not just use `const Type&` as the input to save this check?
##########
src/iceberg/transform_internal.h:
##########
@@ -21,16 +21,22 @@
#include <cstdint>
-namespace iceberg {
+#include "iceberg/result.h"
-/// \brief OS-native thread id for the calling thread, cached per thread.
-///
-/// Returns the cross-process-correlatable id used by spdlog/glog (Linux
-/// gettid, macOS pthread_threadid_np, Windows GetCurrentThreadId), not the
-/// opaque std::thread::id -- and so avoids the std::formatter<std::thread::id>
-/// (P2693) minimum-toolchain dependency. The first call on a thread performs
-/// the OS query; later calls return the cached value (no syscall per call).
-/// Declared here so the platform headers stay confined to the .cc.
-uint64_t OsThreadId() noexcept;
+namespace iceberg::internal {
-} // namespace iceberg
+inline Status ValidateBucketTransformParameter(int32_t num_buckets) {
Review Comment:
Can we avoid adding this new file? We can put these functions into
`src/iceberg/util/transform_util.h`
##########
src/iceberg/transform.cc:
##########
@@ -232,6 +232,18 @@ bool Transform::CanTransform(const Type& source_type)
const {
std::unreachable();
}
+Status Transform::Validate(const std::shared_ptr<Type>& source_type) const {
+ if (!source_type) {
+ return InvalidArgument("Source type cannot be null for transform {}",
ToString());
+ }
+ if (!CanTransform(*source_type)) {
+ return InvalidArgument("Invalid source type {} for transform {}",
+ source_type->ToString(), ToString());
+ }
+ ICEBERG_RETURN_UNEXPECTED(Bind(source_type));
Review Comment:
+1
--
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, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
