Copilot commented on code in PR #433:
URL: https://github.com/apache/fluss-rust/pull/433#discussion_r2907745981
##########
website/docs/user-guide/rust/data-types.md:
##########
@@ -21,6 +21,7 @@ sidebar_position: 3
| `TIMESTAMP_LTZ` | `TimestampLtz` | `get_timestamp_ltz(idx, precision)` |
`set_field(idx, TimestampLtz)` |
| `BYTES` | `&[u8]` | `get_bytes()` |
`set_field(idx, &[u8])` |
| `BINARY(n)` | `&[u8]` | `get_binary(idx, length)` |
`set_field(idx, &[u8])` |
+| `ARRAY<T>` | `FlussArray` | `get_array()` |
`set_field(idx, Datum::Array)` |
Review Comment:
In the data type table, the ARRAY setter is shown as `set_field(idx,
Datum::Array)`, which isn’t a valid call (the variant needs an argument, and
`set_field` accepts `impl Into<Datum>`). Update this entry to reflect an actual
usage, e.g. passing a `FlussArray` (via `Into<Datum>`) or `Datum::Array(arr)`.
##########
crates/fluss/src/row/encode/compacted_key_encoder.rs:
##########
@@ -237,6 +251,51 @@ mod tests {
);
}
+ #[test]
+ fn test_array_type_allowed_as_key() {
+ // Java's CompactedKeyEncoder allows Array as a key column type
+ // (the server rejects unsupported key types at table-creation time).
+ let row_type =
+ RowType::with_data_types(vec![DataTypes::int(),
DataTypes::array(DataTypes::int())]);
+ let mut encoder = CompactedKeyEncoder::new(&row_type, vec![0,
1]).unwrap();
Review Comment:
`test_array_type_allowed_as_key` codifies ARRAY key support, but both the
linked issue #386 and the user guide text in this PR indicate ARRAY should be
rejected as a key column type. Once the intended behavior is decided, adjust
this test accordingly so it doesn’t enforce the opposite contract.
##########
crates/fluss/src/row/compacted/compacted_key_writer.rs:
##########
@@ -47,6 +47,15 @@ impl CompactedKeyWriter {
}
pub fn create_value_writer(field_type: &DataType) -> Result<ValueWriter> {
+ // Java's CompactedKeyEncoder allows encoding Array types (Map/Row
+ // are not yet supported by ValueWriter). The server rejects
+ // unsupported key types at table-creation time, so encoding is
+ // allowed here to match Java parity.
+ if matches!(field_type, DataType::Map(_) | DataType::Row(_)) {
+ return Err(crate::error::Error::IllegalArgument {
+ message: format!("Cannot use {field_type:?} as a key column
type"),
+ });
+ }
Review Comment:
Issue #386 (linked in the PR) calls out that the compacted key encoder
should reject `ARRAY` as a key type, but `create_value_writer` currently only
rejects `Map`/`Row` and explicitly allows `Array` (and tests assert it). Please
confirm the intended contract and either reject `DataType::Array(_)` here (and
update tests/docs), or update the issue/docs to reflect that ARRAY keys are
supported for Java parity.
##########
website/docs/user-guide/rust/data-types.md:
##########
@@ -59,6 +60,29 @@ let data: Vec<Datum> = vec![1i32.into(), "hello".into(),
Datum::Null];
let row = GenericRow::from_data(data);
```
+## Arrays
+
+Use `DataTypes::array(element_type)` in schema definitions. At runtime, read
arrays with `row.get_array(idx)?`.
+
+To construct array values for writes, build a `FlussArray` and wrap it with
`Datum::Array`:
+
+```rust
+use fluss::metadata::DataTypes;
+use fluss::row::binary_array::FlussArrayWriter;
+use fluss::row::{Datum, GenericRow};
+
+let mut writer = FlussArrayWriter::new(3, &DataTypes::int());
+writer.write_int(0, 10);
+writer.write_int(1, 20);
+writer.set_null_at(2);
+let arr = writer.complete()?;
+
+let mut row = GenericRow::new(1);
+row.set_field(0, Datum::Array(arr));
+```
+
+`ARRAY` is supported for row values and nested row fields. Key encoding paths
currently reject `ARRAY`, `MAP`, and `ROW` as key column types.
Review Comment:
This paragraph states that key encoding rejects `ARRAY`, but the Rust key
encoder changes in this PR add `write_array` support and tests that allow
arrays as key columns. Please align documentation with the implemented behavior
(either update docs to say ARRAY keys are allowed, or update the key encoder to
reject ARRAY as described).
--
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]
