felipeblazing commented on a change in pull request #11210:
URL: https://github.com/apache/arrow/pull/11210#discussion_r713921602



##########
File path: cpp/src/arrow/compute/exec/project_node.cc
##########
@@ -91,6 +91,7 @@ class ProjectNode : public ExecNode {
 
   void InputReceived(ExecNode* input, ExecBatch batch) override {
     DCHECK_EQ(input, inputs_[0]);
+    ARROW_LOG(DEBUG) << "ProjectNode: >> project input >>" + batch.ToString();

Review comment:
       I take it this is debugging code specific to your test and this will be 
removed?

##########
File path: cpp/src/arrow/compute/exec/filter_node.cc
##########
@@ -99,11 +105,35 @@ class FilterNode : public ExecNode {
   void InputReceived(ExecNode* input, ExecBatch batch) override {
     DCHECK_EQ(input, inputs_[0]);
 
-    auto maybe_filtered = DoFilter(std::move(batch));
-    if (ErrorIfNotOk(maybe_filtered.status())) return;
+    ARROW_LOG(DEBUG) << "FilterNode: >> input";
+
+    auto executor = plan()->exec_context()->executor();
+    if (executor) {
+      auto maybe_future = executor->Submit([this, batch] {

Review comment:
       I am not saying that we need to include those things now but it would be 
good to see if there is a way to send this information with the task

##########
File path: cpp/src/arrow/compute/exec/filter_node.cc
##########
@@ -99,11 +105,35 @@ class FilterNode : public ExecNode {
   void InputReceived(ExecNode* input, ExecBatch batch) override {
     DCHECK_EQ(input, inputs_[0]);
 
-    auto maybe_filtered = DoFilter(std::move(batch));
-    if (ErrorIfNotOk(maybe_filtered.status())) return;
+    ARROW_LOG(DEBUG) << "FilterNode: >> input";
+
+    auto executor = plan()->exec_context()->executor();

Review comment:
       Is the purpose of the logic here to allow the FilterNode to still be 
used in the old way while allowing it schedule a task as well?

##########
File path: cpp/src/arrow/compute/exec/filter_node.cc
##########
@@ -99,11 +105,35 @@ class FilterNode : public ExecNode {
   void InputReceived(ExecNode* input, ExecBatch batch) override {
     DCHECK_EQ(input, inputs_[0]);
 
-    auto maybe_filtered = DoFilter(std::move(batch));
-    if (ErrorIfNotOk(maybe_filtered.status())) return;
+    ARROW_LOG(DEBUG) << "FilterNode: >> input";
+
+    auto executor = plan()->exec_context()->executor();
+    if (executor) {
+      auto maybe_future = executor->Submit([this, batch] {

Review comment:
       Whatever we submit to the executor must be more than just a lambda if it 
is to encode information that the scheduler needs to use. A cost function and 
context about what query this belongs to and other metadata might want to be 
part of what the task includes.




-- 
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: github-unsubscr...@arrow.apache.org

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


Reply via email to