NGA-TRAN commented on code in PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#discussion_r2135732154
##
datafusion/physical-plan/src/metrics/custom.rs:
##
@@ -0,0 +1,113 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor lic
alamb commented on PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#issuecomment-2950705663
🚀
--
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
alamb merged PR #16195:
URL: https://github.com/apache/datafusion/pull/16195
--
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...@datafusi
alamb commented on PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#issuecomment-2950705278
We have now made the release-48 branch so what is merged into main will be
released as part of DataFusion 49.0.0
--
This is an automated message from the Apache Git Service.
To respo
alamb commented on PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#issuecomment-2945581697
Let's wait to merge this PR until we ship DataFusion 48 to limit the
breaking changes
- #15771
I think we'll be able to merge this in the next few days
--
This is an autom
alamb commented on PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#issuecomment-2944619110
> > Should it target main or the 47 branch ?
>
> The `main` branch is the good one (I don't think the branch-47 is the most
recent release branch anyway)
yeah, let's targe
sfluor commented on PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#issuecomment-2943143826
There was one remaining test failing, I fixed it and updated the PR to the
latest branch. Should it target `main` or the `47` branch ?
--
This is an automated message from the Apach
gabotechs commented on PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#issuecomment-2943183513
> Should it target main or the 47 branch ?
The `main` branch is the good one (I don't the branch-47 is the most recent
release branch anyway)
--
This is an automated messa
sfluor commented on PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#issuecomment-2940051045
Thanks for the review @alamb ! I removed the panic and went with a default
value of 0
--
This is an automated message from the Apache Git Service.
To respond to the message, please
alamb commented on code in PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#discussion_r2126288404
##
datafusion/physical-plan/src/metrics/value.rs:
##
@@ -443,6 +528,9 @@ impl MetricValue {
.and_then(|ts| ts.timestamp_nanos_opt())
alamb commented on code in PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#discussion_r2126279454
##
datafusion/physical-plan/src/metrics/value.rs:
##
@@ -344,7 +344,7 @@ impl Drop for ScopedTimerGuard<'_> {
/// Among other differences, the metric types have dif
LiaCastaneda commented on code in PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#discussion_r2126024222
##
datafusion/physical-plan/src/metrics/value.rs:
##
@@ -443,6 +528,9 @@ impl MetricValue {
.and_then(|ts| ts.timestamp_nanos_opt())
LiaCastaneda commented on code in PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#discussion_r2126024222
##
datafusion/physical-plan/src/metrics/value.rs:
##
@@ -443,6 +528,9 @@ impl MetricValue {
.and_then(|ts| ts.timestamp_nanos_opt())
sfluor commented on code in PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#discussion_r2123513896
##
datafusion/physical-plan/src/metrics/value.rs:
##
@@ -516,6 +596,21 @@ impl MetricValue {
(Self::EndTimestamp(timestamp),
Self::EndTimestamp(other_
gabotechs commented on code in PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#discussion_r2123429852
##
datafusion/physical-plan/src/metrics/value.rs:
##
@@ -516,6 +596,21 @@ impl MetricValue {
(Self::EndTimestamp(timestamp),
Self::EndTimestamp(oth
sfluor commented on PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#issuecomment-2934299401
I have addressed the remaining comments cc @LiaCastaneda / @gabotechs /
@alamb
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on
gabotechs commented on code in PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#discussion_r2120615931
##
datafusion/physical-plan/src/metrics/value.rs:
##
@@ -401,6 +401,90 @@ pub enum MetricValue {
StartTimestamp(Timestamp),
/// The time at which execu
gabotechs commented on code in PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#discussion_r2120632856
##
datafusion/physical-plan/src/metrics/value.rs:
##
@@ -443,6 +528,9 @@ impl MetricValue {
.and_then(|ts| ts.timestamp_nanos_opt())
alamb commented on code in PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#discussion_r2121402980
##
datafusion/physical-plan/src/metrics/value.rs:
##
@@ -344,7 +344,7 @@ impl Drop for ScopedTimerGuard<'_> {
/// Among other differences, the metric types have dif
gabotechs commented on code in PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#discussion_r2120615931
##
datafusion/physical-plan/src/metrics/value.rs:
##
@@ -401,6 +401,90 @@ pub enum MetricValue {
StartTimestamp(Timestamp),
/// The time at which execu
alamb commented on PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#issuecomment-2927412259
@gabotechs / @LiaCastaneda please ping me when you think this PR is ready
for a review / merge
Thank you for the help getting it ready
--
This is an automated message from th
gabotechs commented on code in PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#discussion_r2109744098
##
datafusion/physical-plan/src/metrics/value.rs:
##
@@ -401,6 +401,22 @@ pub enum MetricValue {
StartTimestamp(Timestamp),
/// The time at which execu
sfluor commented on code in PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#discussion_r2110945415
##
datafusion/physical-plan/src/metrics/value.rs:
##
@@ -443,6 +460,9 @@ impl MetricValue {
.and_then(|ts| ts.timestamp_nanos_opt())
gabotechs commented on code in PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#discussion_r2109744098
##
datafusion/physical-plan/src/metrics/value.rs:
##
@@ -401,6 +401,22 @@ pub enum MetricValue {
StartTimestamp(Timestamp),
/// The time at which execu
gabotechs commented on code in PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#discussion_r2109611063
##
datafusion/physical-plan/src/metrics/value.rs:
##
@@ -443,6 +460,9 @@ impl MetricValue {
.and_then(|ts| ts.timestamp_nanos_opt())
LiaCastaneda commented on code in PR #16195:
URL: https://github.com/apache/datafusion/pull/16195#discussion_r2109164412
##
datafusion/physical-plan/src/metrics/value.rs:
##
@@ -589,6 +632,79 @@ mod tests {
use super::*;
+#[derive(Debug, Default)]
+pub struct Cu
sfluor opened a new pull request, #16195:
URL: https://github.com/apache/datafusion/pull/16195
See this issue: https://github.com/apache/datafusion/issues/16044
The MetricValue enum currently exposes only single-value statistics: counts,
gauges, timers, timestamps, and a few hard-code
27 matches
Mail list logo
