Re: [PR] Refactor StreamJoinMetrics to reuse BaselineMetrics [datafusion]

2025-07-05 Thread via GitHub 


alamb commented on PR #16674:
URL: https://github.com/apache/datafusion/pull/16674#issuecomment-3038992838

   Than you @Standing-Man and @2010YOUY01 


-- 
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]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] Refactor StreamJoinMetrics to reuse BaselineMetrics [datafusion]

2025-07-05 Thread via GitHub 


alamb merged PR #16674:
URL: https://github.com/apache/datafusion/pull/16674


-- 
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]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] Refactor StreamJoinMetrics to reuse BaselineMetrics [datafusion]

2025-07-04 Thread via GitHub 


Standing-Man commented on code in PR #16674:
URL: https://github.com/apache/datafusion/pull/16674#discussion_r2184687258


##
datafusion/physical-plan/src/joins/symmetric_hash_join.rs:
##
@@ -1375,7 +1375,7 @@ impl SymmetricHashJoinStream {
 }
 Some((batch, _)) => {
 self.metrics.output_batches.add(1);
-self.metrics.output_rows.add(batch.num_rows());
+self.metrics.baseline_metrics.output_rows();

Review Comment:
   Got it. Will update to use the suggested API for tracking final output rows.



-- 
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]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] Refactor StreamJoinMetrics to reuse BaselineMetrics [datafusion]

2025-07-04 Thread via GitHub 


2010YOUY01 commented on code in PR #16674:
URL: https://github.com/apache/datafusion/pull/16674#discussion_r2184635238


##
datafusion/physical-plan/src/joins/symmetric_hash_join.rs:
##
@@ -1375,7 +1375,7 @@ impl SymmetricHashJoinStream {
 }
 Some((batch, _)) => {
 self.metrics.output_batches.add(1);
-self.metrics.output_rows.add(batch.num_rows());
+self.metrics.baseline_metrics.output_rows();

Review Comment:
   To keep the implementation consistent with the majority of operators, I 
suggest to use the following API to track output rows:
   
https://github.com/apache/datafusion/blob/25c2a079fc1baacc038e304cb2bcae671338c2d8/datafusion/physical-plan/src/metrics/baseline.rs#L124
   This API should be used when the final output is ready to output to the 
downstream operator, and it can also automatically update some other 
`BaselineMetrics`



-- 
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]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

[PR] Refactor StreamJoinMetrics to reuse BaselineMetrics [datafusion]

2025-07-03 Thread via GitHub 


Standing-Man opened a new pull request, #16674:
URL: https://github.com/apache/datafusion/pull/16674

   ## Which issue does this PR close?
   
   
   
   - Closes #16496.
   
   ## Rationale for this change
   
   
   
   Refactor StreamJoinMetrics to reuse BaselineMetrics
   
   ## What changes are included in this PR?
   
   
   
   ## Are these changes tested?
   
   
   
   ## Are there any user-facing changes?
   
   
   
   
   


-- 
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]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]