You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fluss.apache.org by "luoyuxia (via GitHub)" <gi...@apache.org> on 2025/10/16 08:01:56 UTC

[PR] [feat] Support append arrow record batch [fluss-rust]

luoyuxia opened a new pull request, #34:
URL: https://github.com/apache/fluss-rust/pull/34

   <!--
   *Thank you very much for contributing to Fluss - we are happy that you want to help us improve Fluss. To help the community review your contribution in the best possible way, please go through the checklist below, which will get the contribution into a shape in which it can be best reviewed.*
   
   ## Contribution Checklist
   
     - Make sure that the pull request corresponds to a [GitHub issue](https://github.com/apache/fluss-rust/issues). Exceptions are made for typos in JavaDoc or documentation files, which need no issue.
   
     - Name the pull request in the format "[component] Title of the pull request", where *[component]* should be replaced by the name of the component being changed. Typically, this corresponds to the component label assigned to the issue (e.g., [kv], [log], [client], [flink]). Skip *[component]* if you are unsure about which is the best component.
   
     - Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
   
     - Make sure that the change passes the automated tests, i.e., `mvn clean verify` passes.
   
     - Each pull request should address only one issue, not mix up code from multiple issues.
   
   
   **(The sections below can be removed for hotfixes or typos)**
   -->
   
   ### Purpose
   
   <!-- Linking this pull request to the issue -->
   Linked issue: close #xxx
   
   <!-- What is the purpose of the change -->
   
   ### Brief change log
   
   <!-- Please describe the changes made in this pull request and explain how they address the issue -->
   
   ### Tests
   
   <!-- List UT and IT cases to verify this change -->
   
   ### API and Format
   
   <!-- Does this change affect API or storage format -->
   
   ### Documentation
   
   <!-- Does this change introduce a new feature -->
   


-- 
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-unsubscribe@fluss.apache.org

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


Re: [PR] [feat] Support append arrow record batch [fluss-rust]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #34:
URL: https://github.com/apache/fluss-rust/pull/34#discussion_r2434958886


##########
crates/fluss/src/record/arrow.rs:
##########
@@ -95,14 +95,65 @@ pub struct MemoryLogRecordsArrowBuilder {
     magic: u8,
     writer_id: i64,
     batch_sequence: i32,
+    arrow_record_batch_builder: Box<dyn ArrowRecordBatchInnerBuilder>,
+    is_closed: bool,
+}
+
+pub trait ArrowRecordBatchInnerBuilder: Send + Sync {
+    fn build_arrow_record_batch(&self) -> Result<Arc<RecordBatch>>;
+
+    fn append(&mut self, row: &GenericRow) -> Result<bool>;
+
+    fn append_batch(&mut self, record_batch: Arc<RecordBatch>) -> Result<bool>;
+
+    fn schema(&self) -> SchemaRef;
+
+    fn records_count(&self) -> i32;
+
+    fn is_full(&self) -> bool;
+}
+
+#[derive(Default)]
+pub struct DefaultArrowRecordBatchBuilder {
+    arrow_record_batch: Option<Arc<RecordBatch>>,
+    records_count: i32,
+}
+
+impl ArrowRecordBatchInnerBuilder for DefaultArrowRecordBatchBuilder {
+    fn build_arrow_record_batch(&self) -> Result<Arc<RecordBatch>> {
+        Ok(self.arrow_record_batch.as_ref().unwrap().clone())

Review Comment:
   Potential panic if arrow_record_batch is None. Consider using ok_or_else() or returning a proper error instead of unwrap().
   ```suggestion
           self.arrow_record_batch
               .as_ref()
               .cloned()
               .ok_or_else(|| crate::error::Error::msg("arrow_record_batch is None"))
   ```



##########
crates/fluss/src/record/arrow.rs:
##########
@@ -95,14 +95,65 @@ pub struct MemoryLogRecordsArrowBuilder {
     magic: u8,
     writer_id: i64,
     batch_sequence: i32,
+    arrow_record_batch_builder: Box<dyn ArrowRecordBatchInnerBuilder>,
+    is_closed: bool,
+}
+
+pub trait ArrowRecordBatchInnerBuilder: Send + Sync {
+    fn build_arrow_record_batch(&self) -> Result<Arc<RecordBatch>>;
+
+    fn append(&mut self, row: &GenericRow) -> Result<bool>;
+
+    fn append_batch(&mut self, record_batch: Arc<RecordBatch>) -> Result<bool>;
+
+    fn schema(&self) -> SchemaRef;
+
+    fn records_count(&self) -> i32;
+
+    fn is_full(&self) -> bool;
+}
+
+#[derive(Default)]
+pub struct DefaultArrowRecordBatchBuilder {
+    arrow_record_batch: Option<Arc<RecordBatch>>,
+    records_count: i32,
+}
+
+impl ArrowRecordBatchInnerBuilder for DefaultArrowRecordBatchBuilder {
+    fn build_arrow_record_batch(&self) -> Result<Arc<RecordBatch>> {
+        Ok(self.arrow_record_batch.as_ref().unwrap().clone())
+    }
+
+    fn append(&mut self, _row: &GenericRow) -> Result<bool> {
+        Ok(false)
+    }
+
+    fn append_batch(&mut self, record_batch: Arc<RecordBatch>) -> Result<bool> {
+        self.arrow_record_batch = Some(record_batch);
+        Ok(true)
+    }
+
+    fn schema(&self) -> SchemaRef {
+        self.arrow_record_batch.as_ref().unwrap().schema()

Review Comment:
   Potential panic if arrow_record_batch is None. Consider using ok_or_else() or returning a proper error instead of unwrap().
   ```suggestion
       fn schema(&self) -> Result<SchemaRef> {
           self.arrow_record_batch
               .as_ref()
               .map(|batch| batch.schema())
               .ok_or_else(|| crate::error::Error::ArrowRecordBatchNotSet)
   ```



##########
crates/fluss/src/client/write/mod.rs:
##########
@@ -34,13 +35,42 @@ mod writer_client;
 pub use writer_client::WriterClient;
 
 pub struct WriteRecord<'a> {
-    pub row: GenericRow<'a>,
+    pub row: Record<'a>,
     pub table_path: Arc<TablePath>,
 }
 
+pub enum Record<'a> {
+    Row(GenericRow<'a>),
+    RecordBatch(Arc<RecordBatch>),
+}
+
 impl<'a> WriteRecord<'a> {
     pub fn new(table_path: Arc<TablePath>, row: GenericRow<'a>) -> Self {
-        Self { row, table_path }
+        Self {
+            row: Record::Row(row),
+            table_path,
+        }
+    }
+
+    pub fn new_record_batch(table_path: Arc<TablePath>, row: RecordBatch) -> Self {
+        Self {
+            row: Record::RecordBatch(Arc::new(row)),
+            table_path,
+        }
+    }
+}
+
+pub struct WriteRecordBatch {
+    pub record_batch: RecordBatch,
+    pub table_path: Arc<TablePath>,
+}
+
+impl WriteRecordBatch {
+    pub fn new(table_path: Arc<TablePath>, record_batch: RecordBatch) -> Self {
+        Self {
+            record_batch,
+            table_path,
+        }
     }
 }

Review Comment:
   The WriteRecordBatch struct and its implementation appear to be unused code. Consider removing it or adding proper usage if it's intended for future use.
   ```suggestion
   
   ```



-- 
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-unsubscribe@fluss.apache.org

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


Re: [PR] [feat] Support append arrow record batch [fluss-rust]

Posted by "naivedogger (via GitHub)" <gi...@apache.org>.
naivedogger commented on code in PR #34:
URL: https://github.com/apache/fluss-rust/pull/34#discussion_r2446777791


##########
crates/fluss/tests/integration/admin.rs:
##########
@@ -43,16 +43,19 @@ mod admin_test {
     fn before_all() {
         // Create a new tokio runtime in a separate thread
         let cluster_guard = SHARED_FLUSS_CLUSTER.clone();
-        std::thread::spawn(move || {
+        thread::spawn(move || {
             let rt = tokio::runtime::Runtime::new().expect("Failed to create runtime");
             rt.block_on(async {
-                let cluster = FlussTestingClusterBuilder::new().build().await;
+                let cluster = FlussTestingClusterBuilder::new("test-admin").build().await;
                 let mut guard = cluster_guard.write();
                 *guard = Some(cluster);
             });
         })
         .join()
         .expect("Failed to create cluster");
+        // wait for 10 seconds to avoid the error like

Review Comment:
   `//wait for 20 seconds to avoid the error like`
   Maybe use a const?



##########
crates/fluss/tests/integration/table.rs:
##########
@@ -36,25 +36,25 @@ mod table_test {
     use crate::integration::fluss_cluster::{FlussTestingCluster, FlussTestingClusterBuilder};
     use crate::integration::utils::create_table;
     use arrow::array::record_batch;
-    use fluss::metadata::{
-        DataTypes, DatabaseDescriptorBuilder, KvFormat, LogFormat, Schema, TableDescriptor,
-        TablePath,
-    };
+    use fluss::metadata::{DataTypes, Schema, TableDescriptor, TablePath};
     use std::sync::Arc;
+    use std::sync::atomic::AtomicUsize;
     use std::thread;
     fn before_all() {
         // Create a new tokio runtime in a separate thread
         let cluster_guard = SHARED_FLUSS_CLUSTER.clone();
         std::thread::spawn(move || {
             let rt = tokio::runtime::Runtime::new().expect("Failed to create runtime");
             rt.block_on(async {
-                let cluster = FlussTestingClusterBuilder::new().build().await;
+                let cluster = FlussTestingClusterBuilder::new("test_table").build().await;
                 let mut guard = cluster_guard.write();
                 *guard = Some(cluster);
             });
         })
         .join()
         .expect("Failed to create cluster");
+        // wait for 10 seconds to avoid the error like

Review Comment:
   ditto



-- 
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-unsubscribe@fluss.apache.org

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


Re: [PR] [feat] Support append arrow record batch [fluss-rust]

Posted by "beryllw (via GitHub)" <gi...@apache.org>.
beryllw commented on PR #34:
URL: https://github.com/apache/fluss-rust/pull/34#issuecomment-3424363767

   LGTM!


-- 
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-unsubscribe@fluss.apache.org

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


Re: [PR] [feat] Support append arrow record batch [fluss-rust]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #34:
URL: https://github.com/apache/fluss-rust/pull/34#discussion_r2444993974


##########
crates/fluss/src/record/arrow.rs:
##########
@@ -95,14 +95,71 @@ pub struct MemoryLogRecordsArrowBuilder {
     magic: u8,
     writer_id: i64,
     batch_sequence: i32,
+    arrow_record_batch_builder: Box<dyn ArrowRecordBatchInnerBuilder>,
+    is_closed: bool,
+}
+
+pub trait ArrowRecordBatchInnerBuilder: Send + Sync {
+    fn build_arrow_record_batch(&self) -> Result<Arc<RecordBatch>>;
+
+    fn append(&mut self, row: &GenericRow) -> Result<bool>;
+
+    fn append_batch(&mut self, record_batch: Arc<RecordBatch>) -> Result<bool>;
+
+    fn schema(&self) -> SchemaRef;
+
+    fn records_count(&self) -> i32;
+
+    fn is_full(&self) -> bool;
+}
+
+#[derive(Default)]
+pub struct PrebuiltRecordBatchBuilder {
+    arrow_record_batch: Option<Arc<RecordBatch>>,
+    records_count: i32,
+}
+
+impl ArrowRecordBatchInnerBuilder for PrebuiltRecordBatchBuilder {
+    fn build_arrow_record_batch(&self) -> Result<Arc<RecordBatch>> {
+        Ok(self.arrow_record_batch.as_ref().unwrap().clone())
+    }
+
+    fn append(&mut self, _row: &GenericRow) -> Result<bool> {
+        // append one single row is not supported, return false directly
+        Ok(false)
+    }
+
+    fn append_batch(&mut self, record_batch: Arc<RecordBatch>) -> Result<bool> {
+        if self.arrow_record_batch.is_some() {
+            return Ok(false);
+        }
+        self.records_count = record_batch.num_rows() as i32;
+        self.arrow_record_batch = Some(record_batch);
+        Ok(true)
+    }
+
+    fn schema(&self) -> SchemaRef {
+        self.arrow_record_batch.as_ref().unwrap().schema()

Review Comment:
   The code uses `unwrap()` on Option without proper validation. The `build_arrow_record_batch` and `schema` methods can panic if `arrow_record_batch` is None. Add proper error handling or ensure these methods are only called after successful append_batch.



##########
crates/fluss/tests/integration/table.rs:
##########
@@ -0,0 +1,134 @@
+/*
+ * 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.
+ */
+
+use crate::integration::fluss_cluster::FlussTestingCluster;
+use once_cell::sync::Lazy;
+use parking_lot::RwLock;
+use std::sync::Arc;
+
+#[cfg(test)]
+use test_env_helpers::*;
+
+// Module-level shared cluster instance (only for this test file)
+static SHARED_FLUSS_CLUSTER: Lazy<Arc<RwLock<Option<FlussTestingCluster>>>> =
+    Lazy::new(|| Arc::new(RwLock::new(None)));
+
+#[cfg(test)]
+#[before_all]
+#[after_all]
+mod table_test {
+    use super::SHARED_FLUSS_CLUSTER;
+    use crate::integration::fluss_cluster::{FlussTestingCluster, FlussTestingClusterBuilder};
+    use crate::integration::utils::create_table;
+    use arrow::array::record_batch;
+    use fluss::metadata::{
+        DataTypes, DatabaseDescriptorBuilder, KvFormat, LogFormat, Schema, TableDescriptor,
+        TablePath,
+    };
+    use std::sync::Arc;
+    use std::thread;
+    fn before_all() {
+        // Create a new tokio runtime in a separate thread
+        let cluster_guard = SHARED_FLUSS_CLUSTER.clone();
+        std::thread::spawn(move || {
+            let rt = tokio::runtime::Runtime::new().expect("Failed to create runtime");
+            rt.block_on(async {
+                let cluster = FlussTestingClusterBuilder::new("test_table").build().await;
+                let mut guard = cluster_guard.write();
+                *guard = Some(cluster);
+            });
+        })
+        .join()
+        .expect("Failed to create cluster");
+        // wait for 10 seconds to avoid the error like
+        // CoordinatorEventProcessor is not initialized yet
+        thread::sleep(std::time::Duration::from_secs(20));

Review Comment:
   Hard-coded 20-second sleep is excessive and makes tests slower. Consider using a shorter duration or implementing a retry mechanism to check for readiness.



##########
.github/workflows/ci.yml:
##########
@@ -91,7 +91,7 @@ jobs:
         # only run IT in linux since no docker in macos by default
         run: |
           if [ "$RUNNER_OS" == "Linux" ]; then
-            cargo test --features integration_tests --all-targets --workspace
+            RUST_TEST_THREADS=1 cargo test --features integration_tests --all-targets --workspace  -- --nocapture

Review Comment:
   Setting RUST_TEST_THREADS=1 forces all tests to run sequentially, which significantly slows down CI execution. Consider using a higher value or implementing proper test isolation instead.
   ```suggestion
               cargo test --features integration_tests --all-targets --workspace  -- --nocapture
   ```



-- 
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-unsubscribe@fluss.apache.org

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


Re: [PR] [feat] Support append arrow record batch [fluss-rust]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #34:
URL: https://github.com/apache/fluss-rust/pull/34#discussion_r2444079979


##########
crates/fluss/src/record/arrow.rs:
##########
@@ -95,14 +95,67 @@ pub struct MemoryLogRecordsArrowBuilder {
     magic: u8,
     writer_id: i64,
     batch_sequence: i32,
+    arrow_record_batch_builder: Box<dyn ArrowRecordBatchInnerBuilder>,
+    is_closed: bool,
+}
+
+pub trait ArrowRecordBatchInnerBuilder: Send + Sync {
+    fn build_arrow_record_batch(&self) -> Result<Arc<RecordBatch>>;
+
+    fn append(&mut self, row: &GenericRow) -> Result<bool>;
+
+    fn append_batch(&mut self, record_batch: Arc<RecordBatch>) -> Result<bool>;
+
+    fn schema(&self) -> SchemaRef;
+
+    fn records_count(&self) -> i32;
+
+    fn is_full(&self) -> bool;
+}
+
+#[derive(Default)]
+pub struct DefaultArrowRecordBatchBuilder {
+    arrow_record_batch: Option<Arc<RecordBatch>>,
+    records_count: i32,
+}
+
+impl ArrowRecordBatchInnerBuilder for DefaultArrowRecordBatchBuilder {
+    fn build_arrow_record_batch(&self) -> Result<Arc<RecordBatch>> {
+        Ok(self.arrow_record_batch.as_ref().unwrap().clone())
+    }
+
+    fn append(&mut self, _row: &GenericRow) -> Result<bool> {
+        // append one single row is not supported, return false directly
+        Ok(false)
+    }
+
+    fn append_batch(&mut self, record_batch: Arc<RecordBatch>) -> Result<bool> {
+        self.arrow_record_batch = Some(record_batch);
+        Ok(true)
+    }
+
+    fn schema(&self) -> SchemaRef {
+        self.arrow_record_batch.as_ref().unwrap().schema()

Review Comment:
   Potential panic if `arrow_record_batch` is None. The method should return an error or ensure the batch exists before unwrapping.



##########
crates/fluss/src/client/write/writer_client.rs:
##########
@@ -121,6 +113,21 @@ impl WriterClient {
 
         Ok(result.result_handle.expect("result_handle should exist"))
     }
+    fn assign_bucket(&self, table_path: &Arc<TablePath>) -> (Arc<Box<dyn BucketAssigner>>, i32) {

Review Comment:
   The return type `Arc<Box<dyn BucketAssigner>>` involves double indirection. Consider returning `(Arc<dyn BucketAssigner>, i32)` instead for better performance and cleaner code.



##########
crates/fluss/tests/integration/table.rs:
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.
+ */
+
+use crate::integration::fluss_cluster::FlussTestingCluster;
+use once_cell::sync::Lazy;
+use parking_lot::RwLock;
+use std::sync::Arc;
+
+#[cfg(test)]
+use test_env_helpers::*;
+
+// Module-level shared cluster instance (only for this test file)
+static SHARED_FLUSS_CLUSTER: Lazy<Arc<RwLock<Option<FlussTestingCluster>>>> =
+    Lazy::new(|| Arc::new(RwLock::new(None)));
+
+#[cfg(test)]
+#[before_all]
+#[after_all]
+mod table_test {
+    use super::SHARED_FLUSS_CLUSTER;
+    use crate::integration::fluss_cluster::{FlussTestingCluster, FlussTestingClusterBuilder};
+    use crate::integration::utils::create_table;
+    use arrow::array::record_batch;
+    use fluss::metadata::{
+        DataTypes, DatabaseDescriptorBuilder, KvFormat, LogFormat, Schema, TableDescriptor,
+        TablePath,
+    };
+    use std::sync::Arc;
+    use std::thread;
+    fn before_all() {
+        // Create a new tokio runtime in a separate thread
+        let cluster_guard = SHARED_FLUSS_CLUSTER.clone();
+        std::thread::spawn(move || {
+            let rt = tokio::runtime::Runtime::new().expect("Failed to create runtime");
+            rt.block_on(async {
+                let cluster = FlussTestingClusterBuilder::new().build().await;
+                let mut guard = cluster_guard.write();
+                *guard = Some(cluster);
+            });
+        })
+        .join()
+        .expect("Failed to create cluster");
+        thread::sleep(std::time::Duration::from_secs(20));

Review Comment:
   Hard-coded 20-second sleep is inefficient and makes tests slow. Consider using a polling mechanism or wait condition instead of a fixed delay.



-- 
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-unsubscribe@fluss.apache.org

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


Re: [PR] [feat] Support append arrow record batch [fluss-rust]

Posted by "beryllw (via GitHub)" <gi...@apache.org>.
beryllw commented on code in PR #34:
URL: https://github.com/apache/fluss-rust/pull/34#discussion_r2446509631


##########
crates/fluss/tests/integration/fluss_cluster.rs:
##########
@@ -83,15 +97,25 @@ impl FlussTestingClusterBuilder {
 
     async fn start_coordinator_server(&mut self) -> ContainerAsync<GenericImage> {
         let mut coordinator_confs = HashMap::new();
-        coordinator_confs.insert("zookeeper.address", "zookeeper:2181");
+        coordinator_confs.insert(
+            "zookeeper.address",
+            format!("{}:2181", self.zookeeper_container_name()),
+        );
         coordinator_confs.insert(
             "bind.listeners",
-            "INTERNAL://coordinator-server:0, CLIENT://coordinator-server:9123",
+            format!(
+                "INTERNAL://{}:0, CLIENT://{}:9123",
+                self.coordinator_server_container_name(),
+                self.coordinator_server_container_name()
+            ),
         );
-        coordinator_confs.insert("advertised.listeners", "CLIENT://localhost:9123");
-        coordinator_confs.insert("internal.listener.name", "INTERNAL");
+        coordinator_confs.insert(
+            "advertised.listeners",
+            "CLIENT://localhost:9123".to_string(),
+        );
+        coordinator_confs.insert("internal.listener.name", "INTERNAL".to_string());
         GenericImage::new("fluss/fluss", FLUSS_VERSION)
-            .with_container_name("coordinator-server")
+            .with_container_name(format!("coordinator-server-{}", self.testing_name))

Review Comment:
   We could use ` .with_container_name(self.coordinator_server_container_name().as_str())`



-- 
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-unsubscribe@fluss.apache.org

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


Re: [PR] [feat] Support append arrow record batch [fluss-rust]

Posted by "beryllw (via GitHub)" <gi...@apache.org>.
beryllw commented on code in PR #34:
URL: https://github.com/apache/fluss-rust/pull/34#discussion_r2446513371


##########
crates/fluss/tests/integration/table.rs:
##########
@@ -0,0 +1,134 @@
+/*
+ * 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.
+ */
+
+use crate::integration::fluss_cluster::FlussTestingCluster;
+use once_cell::sync::Lazy;
+use parking_lot::RwLock;
+use std::sync::Arc;
+
+#[cfg(test)]
+use test_env_helpers::*;
+
+// Module-level shared cluster instance (only for this test file)
+static SHARED_FLUSS_CLUSTER: Lazy<Arc<RwLock<Option<FlussTestingCluster>>>> =
+    Lazy::new(|| Arc::new(RwLock::new(None)));
+
+#[cfg(test)]
+#[before_all]
+#[after_all]
+mod table_test {
+    use super::SHARED_FLUSS_CLUSTER;
+    use crate::integration::fluss_cluster::{FlussTestingCluster, FlussTestingClusterBuilder};
+    use crate::integration::utils::create_table;
+    use arrow::array::record_batch;
+    use fluss::metadata::{
+        DataTypes, DatabaseDescriptorBuilder, KvFormat, LogFormat, Schema, TableDescriptor,

Review Comment:
   ```
   warning: unused imports: `DatabaseDescriptorBuilder`, `KvFormat`, and `LogFormat`
     --> crates/fluss/tests/integration/table.rs:40:20
      |
   40 |         DataTypes, DatabaseDescriptorBuilder, KvFormat, LogFormat, Schema, TableDescriptor,
      |                    ^^^^^^^^^^^^^^^^^^^^^^^^^  ^^^^^^^^  ^^^^^^^^^
      |
      = note: `#[warn(unused_imports)]` on by default
   ```



-- 
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-unsubscribe@fluss.apache.org

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


Re: [PR] [feat] Support append arrow record batch [fluss-rust]

Posted by "naivedogger (via GitHub)" <gi...@apache.org>.
naivedogger commented on PR #34:
URL: https://github.com/apache/fluss-rust/pull/34#issuecomment-3425036746

   +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: issues-unsubscribe@fluss.apache.org

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


Re: [PR] [feat] Support append arrow record batch [fluss-rust]

Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia merged PR #34:
URL: https://github.com/apache/fluss-rust/pull/34


-- 
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-unsubscribe@fluss.apache.org

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