zhjwpku commented on code in PR #117:
URL: https://github.com/apache/iceberg-cpp/pull/117#discussion_r2132010329


##########
src/iceberg/literal.h:
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+#include <compare>
+#include <memory>
+#include <string>
+#include <variant>
+#include <vector>
+
+#include "iceberg/result.h"
+#include "iceberg/type.h"
+
+namespace iceberg {
+
+/// \brief PrimitiveLiteral is owned literal of a primitive type.
+class ICEBERG_EXPORT PrimitiveLiteral {
+ private:
+  /// \brief Exception type for values that are below the minimum allowed 
value for a
+  /// primitive type.
+  ///
+  /// When casting a value to a narrow primitive type, if the value exceeds 
the maximum of
+  /// dest type, it might be above the maximum allowed value for that type.
+  struct BelowMin {
+    bool operator==(const BelowMin&) const = default;
+    std::strong_ordering operator<=>(const BelowMin&) const = default;
+  };
+
+  /// \brief Exception type for values that are above the maximum allowed 
value for a
+  /// primitive type.
+  ///
+  /// When casting a value to a narrow primitive type, if the value exceeds 
the maximum of
+  /// dest type, it might be above the maximum allowed value for that type.
+  struct AboveMax {
+    bool operator==(const AboveMax&) const = default;
+    std::strong_ordering operator<=>(const AboveMax&) const = default;
+  };
+
+  using PrimitiveLiteralValue =
+      std::variant<bool,                     // for boolean
+                   int32_t,                  // for int, date
+                   int64_t,                  // for long, timestamp, 
timestamp_tz, time
+                   float,                    // for float
+                   double,                   // for double
+                   std::string,              // for string
+                   std::vector<uint8_t>,     // for binary, fixed
+                   std::array<uint8_t, 16>,  // for uuid and decimal
+                   BelowMin, AboveMax>;
+
+ public:
+  /// Factory methods for primitive types
+  static PrimitiveLiteral Boolean(bool value);
+  static PrimitiveLiteral Int(int32_t value);
+  static PrimitiveLiteral Long(int64_t value);
+  static PrimitiveLiteral Float(float value);
+  static PrimitiveLiteral Double(double value);
+  static PrimitiveLiteral String(std::string value);
+  static PrimitiveLiteral Binary(std::vector<uint8_t> value);
+
+  /// Create iceberg value from bytes.
+  ///
+  /// See [this 
spec](https://iceberg.apache.org/spec/#binary-single-value-serialization)
+  /// for reference.
+  static Result<PrimitiveLiteral> Deserialize(std::span<const uint8_t> data);
+  /// Serialize iceberg value to bytes.

Review Comment:
   nit: add an empty line after a method?



##########
src/iceberg/literal.h:
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+#include <compare>
+#include <memory>
+#include <string>
+#include <variant>
+#include <vector>
+
+#include "iceberg/result.h"
+#include "iceberg/type.h"
+
+namespace iceberg {
+
+/// \brief PrimitiveLiteral is owned literal of a primitive type.

Review Comment:
   I had some trouble understanding this sentence, how about:
   
   ```
   PrimitiveLiteral is a literal value that is associated with a primitive type.
   ```



##########
src/iceberg/literal.h:
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+#include <compare>
+#include <memory>
+#include <string>
+#include <variant>
+#include <vector>
+
+#include "iceberg/result.h"
+#include "iceberg/type.h"
+
+namespace iceberg {
+
+/// \brief PrimitiveLiteral is owned literal of a primitive type.
+class ICEBERG_EXPORT PrimitiveLiteral {
+ private:
+  /// \brief Exception type for values that are below the minimum allowed 
value for a
+  /// primitive type.
+  ///
+  /// When casting a value to a narrow primitive type, if the value exceeds 
the maximum of
+  /// dest type, it might be above the maximum allowed value for that type.
+  struct BelowMin {
+    bool operator==(const BelowMin&) const = default;
+    std::strong_ordering operator<=>(const BelowMin&) const = default;
+  };
+
+  /// \brief Exception type for values that are above the maximum allowed 
value for a
+  /// primitive type.
+  ///
+  /// When casting a value to a narrow primitive type, if the value exceeds 
the maximum of
+  /// dest type, it might be above the maximum allowed value for that type.
+  struct AboveMax {
+    bool operator==(const AboveMax&) const = default;
+    std::strong_ordering operator<=>(const AboveMax&) const = default;
+  };
+
+  using PrimitiveLiteralValue =
+      std::variant<bool,                     // for boolean
+                   int32_t,                  // for int, date
+                   int64_t,                  // for long, timestamp, 
timestamp_tz, time
+                   float,                    // for float
+                   double,                   // for double
+                   std::string,              // for string
+                   std::vector<uint8_t>,     // for binary, fixed
+                   std::array<uint8_t, 16>,  // for uuid and decimal
+                   BelowMin, AboveMax>;
+
+ public:
+  /// Factory methods for primitive types
+  static PrimitiveLiteral Boolean(bool value);
+  static PrimitiveLiteral Int(int32_t value);
+  static PrimitiveLiteral Long(int64_t value);
+  static PrimitiveLiteral Float(float value);
+  static PrimitiveLiteral Double(double value);
+  static PrimitiveLiteral String(std::string value);
+  static PrimitiveLiteral Binary(std::vector<uint8_t> value);
+
+  /// Create iceberg value from bytes.

Review Comment:
   ```suggestion
     /// Create iceberg Literal from bytes.
   ```



##########
src/iceberg/literal.h:
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+#include <compare>
+#include <memory>
+#include <string>
+#include <variant>
+#include <vector>
+
+#include "iceberg/result.h"
+#include "iceberg/type.h"
+
+namespace iceberg {
+
+/// \brief PrimitiveLiteral is owned literal of a primitive type.
+class ICEBERG_EXPORT PrimitiveLiteral {
+ private:
+  /// \brief Exception type for values that are below the minimum allowed 
value for a
+  /// primitive type.
+  ///
+  /// When casting a value to a narrow primitive type, if the value exceeds 
the maximum of
+  /// dest type, it might be above the maximum allowed value for that type.
+  struct BelowMin {
+    bool operator==(const BelowMin&) const = default;
+    std::strong_ordering operator<=>(const BelowMin&) const = default;
+  };
+
+  /// \brief Exception type for values that are above the maximum allowed 
value for a
+  /// primitive type.
+  ///
+  /// When casting a value to a narrow primitive type, if the value exceeds 
the maximum of
+  /// dest type, it might be above the maximum allowed value for that type.
+  struct AboveMax {
+    bool operator==(const AboveMax&) const = default;
+    std::strong_ordering operator<=>(const AboveMax&) const = default;
+  };
+
+  using PrimitiveLiteralValue =
+      std::variant<bool,                     // for boolean
+                   int32_t,                  // for int, date
+                   int64_t,                  // for long, timestamp, 
timestamp_tz, time
+                   float,                    // for float
+                   double,                   // for double
+                   std::string,              // for string
+                   std::vector<uint8_t>,     // for binary, fixed
+                   std::array<uint8_t, 16>,  // for uuid and decimal
+                   BelowMin, AboveMax>;
+
+ public:
+  /// Factory methods for primitive types
+  static PrimitiveLiteral Boolean(bool value);
+  static PrimitiveLiteral Int(int32_t value);
+  static PrimitiveLiteral Long(int64_t value);
+  static PrimitiveLiteral Float(float value);
+  static PrimitiveLiteral Double(double value);
+  static PrimitiveLiteral String(std::string value);
+  static PrimitiveLiteral Binary(std::vector<uint8_t> value);
+
+  /// Create iceberg value from bytes.
+  ///
+  /// See [this 
spec](https://iceberg.apache.org/spec/#binary-single-value-serialization)
+  /// for reference.
+  static Result<PrimitiveLiteral> Deserialize(std::span<const uint8_t> data);
+  /// Serialize iceberg value to bytes.
+  ///
+  /// See [this 
spec](https://iceberg.apache.org/spec/#binary-single-value-serialization)
+  /// for reference.
+  Result<std::vector<uint8_t>> Serialize() const;
+
+  /// Get the Iceberg Type of the literal
+  const std::shared_ptr<PrimitiveType>& type() const;
+
+  /// Converts this literal to a literal of the given type.
+  ///
+  /// When a predicate is bound to a concrete data column, literals are 
converted to match
+  /// the bound column's type. This conversion process is more narrow than a 
cast and is
+  /// only intended for cases where substituting one type is a common mistake 
(e.g. 34
+  /// instead of 34L) or where this API avoids requiring a concrete class 
(e.g., dates).
+  ///
+  /// If conversion to a target type is not supported, this method returns an 
error.
+  ///
+  /// This method may return BelowMin or AboveMax when the target type is not 
as wide as
+  /// the original type. These values indicate that the containing predicate 
can be
+  /// simplified. For example, Integer.MAX_VALUE+1 converted to an int will 
result in
+  /// AboveMax and can simplify a < Integer.MAX_VALUE+1 to always true.
+  ///
+  /// @param target_type A primitive PrimitiveType
+  /// @return A Result containing a literal of the given type or an error if 
conversion

Review Comment:
   ```suggestion
     /// \param target_type A primitive PrimitiveType
     /// \return A Result containing a literal of the given type or an error if 
conversion
   ```



##########
src/iceberg/literal.h:
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+#include <compare>
+#include <memory>
+#include <string>
+#include <variant>
+#include <vector>
+
+#include "iceberg/result.h"
+#include "iceberg/type.h"
+
+namespace iceberg {
+
+/// \brief PrimitiveLiteral is owned literal of a primitive type.
+class ICEBERG_EXPORT PrimitiveLiteral {
+ private:
+  /// \brief Exception type for values that are below the minimum allowed 
value for a
+  /// primitive type.
+  ///
+  /// When casting a value to a narrow primitive type, if the value exceeds 
the maximum of
+  /// dest type, it might be above the maximum allowed value for that type.

Review Comment:
   ```suggestion
     /// target type, it might be above the maximum allowed value for that type.
   ```



##########
src/iceberg/literal.h:
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+#include <compare>
+#include <memory>
+#include <string>
+#include <variant>
+#include <vector>
+
+#include "iceberg/result.h"
+#include "iceberg/type.h"
+
+namespace iceberg {
+
+/// \brief PrimitiveLiteral is owned literal of a primitive type.
+class ICEBERG_EXPORT PrimitiveLiteral {
+ private:
+  /// \brief Exception type for values that are below the minimum allowed 
value for a
+  /// primitive type.
+  ///
+  /// When casting a value to a narrow primitive type, if the value exceeds 
the maximum of
+  /// dest type, it might be above the maximum allowed value for that type.
+  struct BelowMin {
+    bool operator==(const BelowMin&) const = default;
+    std::strong_ordering operator<=>(const BelowMin&) const = default;
+  };
+
+  /// \brief Exception type for values that are above the maximum allowed 
value for a
+  /// primitive type.
+  ///
+  /// When casting a value to a narrow primitive type, if the value exceeds 
the maximum of
+  /// dest type, it might be above the maximum allowed value for that type.

Review Comment:
   ```suggestion
     /// target type, it might be above the maximum allowed value for that type.
   ```



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to