luoyuxia commented on code in PR #265:
URL: https://github.com/apache/fluss-rust/pull/265#discussion_r2778373654
##########
bindings/cpp/include/fluss.hpp:
##########
@@ -182,7 +187,14 @@ struct Schema {
public:
Builder& AddColumn(std::string name, DataType type,
std::string comment = "") {
- columns_.push_back({std::move(name), type, std::move(comment)});
+ columns_.push_back({std::move(name), type, std::move(comment), 0,
0});
+ return *this;
+ }
+
+ Builder& AddDecimalColumn(std::string name, int32_t precision, int32_t
scale,
Review Comment:
Also, I think timestamp support can also pass precision since the default
precision is 6, but the wide-used is 3(for mill-seconds)
##########
bindings/cpp/include/fluss.hpp:
##########
@@ -182,7 +187,14 @@ struct Schema {
public:
Builder& AddColumn(std::string name, DataType type,
std::string comment = "") {
- columns_.push_back({std::move(name), type, std::move(comment)});
+ columns_.push_back({std::move(name), type, std::move(comment), 0,
0});
+ return *this;
+ }
+
+ Builder& AddDecimalColumn(std::string name, int32_t precision, int32_t
scale,
Review Comment:
I found for decimal type, we need another method which may not normal to
users. And current design make it harder to support `map`, `array`, `nestrow`
type.
Duckdb is a good reference, but may be we can have a simple way:
```
enum class TypeId {
Boolean, Int, BigInt, Float, Double, String, Bytes,
Date, Time, Timestamp, Decimal
};
class DataType {
public:
static DataType Int() { return DataType(TypeId::Int); }
static DataType BigInt() { return DataType(TypeId::BigInt); }
static DataType String() { return DataType(TypeId::String); }
static DataType Boolean() { return DataType(TypeId::Boolean); }
static DataType Double() { return DataType(TypeId::Double); }
static DataType Decimal(int32_t precision, int32_t scale) {
return DataType(TypeId::Decimal, precision, scale);
}
TypeId id() const { return id_; }
int32_t precision() const { return precision_; }
int32_t scale() const { return scale_; }
private:
explicit DataType(TypeId id, int32_t p = 0, int32_t s = 0)
: id_(id), precision_(p), scale_(s) {}
TypeId id_;
int32_t precision_;
int32_t scale_;
};
```
And we can still improve the internal implementation since it's we can keep
user-face api not changed.
##########
bindings/cpp/include/fluss.hpp:
##########
@@ -290,6 +302,10 @@ struct Datum {
double f64_val{0.0};
std::string string_val;
std::vector<uint8_t> bytes_val;
+ int32_t decimal_precision{0};
Review Comment:
nit: add comments for these 4 field to explain they are for decimal value?
--
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]
