wgtmac commented on code in PR #156:
URL: https://github.com/apache/iceberg-cpp/pull/156#discussion_r2240139253
##########
src/iceberg/transform.h:
##########
@@ -172,6 +174,8 @@ class ICEBERG_EXPORT TransformFunction {
TransformFunction(TransformType transform_type, std::shared_ptr<Type>
source_type);
/// \brief Transform an input array to a new array
virtual Result<ArrowArray> Transform(const ArrowArray& data) = 0;
Review Comment:
Perhaps we can remove this now? cc @gty404
##########
src/iceberg/expression/literal.h:
##########
@@ -63,7 +63,10 @@ class ICEBERG_EXPORT Literal {
/// \brief Factory methods for primitive types
static Literal Boolean(bool value);
static Literal Int(int32_t value);
+ static Literal Date(int32_t value);
static Literal Long(int64_t value);
+ static Literal Timestamp(int64_t value);
Review Comment:
Do we need to add `Time`?
##########
src/iceberg/expression/literal.cc:
##########
@@ -208,12 +216,30 @@ std::partial_ordering Literal::operator<=>(const Literal&
other) const {
return this_val <=> other_val;
}
+ case TypeId::kDate: {
Review Comment:
Should we merge it with the above case? We can add [[fallthrough]] to make
it explicit.
##########
src/iceberg/util/macros.h:
##########
@@ -36,3 +36,8 @@
#define ICEBERG_ASSIGN_OR_RAISE(lhs, rexpr)
\
ICEBERG_ASSIGN_OR_RAISE_IMPL(ICEBERG_ASSIGN_OR_RAISE_NAME(result_,
__COUNTER__), lhs, \
rexpr)
+
+#define NANOARROW_RETURN_IF_NOT_OK(status, error) \
+ if (status != NANOARROW_OK) [[unlikely]] { \
Review Comment:
We shouldn't expose nanoarrow publicly.
##########
src/iceberg/expression/literal.h:
##########
@@ -85,6 +88,9 @@ class ICEBERG_EXPORT Literal {
/// \brief Get the literal type.
const std::shared_ptr<PrimitiveType>& type() const;
+ /// \brief Get the literal value.
+ const Value& value() const { return value_; }
Review Comment:
`Value` is still private. We might need to make it public.
--
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]
