Use TwoFloats when using stats_agg in moving-aggregate
#599
Conversation
|
|
||
| // expanded from FlatSerializable derive macro and made to work right with generic arg | ||
| #[allow(unused_imports)] | ||
| unsafe impl<'a> flat_serialize::FlatSerializable<'a> for StatsSummary2D<f64> { |
There was a problem hiding this comment.
#[derive(FlatSerializable)] doesn't seem to be able to handle types with generics, so I manually expanded it and fixed the output.
There was a problem hiding this comment.
Would it be cleaner to use the TypeHack workaround you use below?
There was a problem hiding this comment.
Rust doesn't support applying derive macros on type aliases so that wouldn't work here. The ideal solution would be to fix the derive macro.
syvb
force-pushed
the
sv/stats_agg-twofloat
branch
from
046f2cb to
dce3089
Compare
|
|
||
| // expanded from FlatSerializable derive macro and made to work right with generic arg | ||
| #[allow(unused_imports)] | ||
| unsafe impl<'a> flat_serialize::FlatSerializable<'a> for StatsSummary2D<f64> { |
There was a problem hiding this comment.
Would it be cleaner to use the TypeHack workaround you use below?
| } | ||
|
|
||
| fn pg2d_agg(agg: &str) -> String { | ||
| format!("SELECT {}(test_y, test_x) FROM test_table", agg) | ||
| format!("SELECT {agg}(test_y, test_x), (SELECT {agg}(test_y, test_x) OVER (ORDER BY test_x ROWS BETWEEN 1 PRECEDING AND 1 FOLLOWING) FROM test_table LIMIT 1 OFFSET 3) FROM test_table", agg = agg) |
There was a problem hiding this comment.
I don't think this is going to be enough rows to actually see a difference and possibly will never even call the twofloat version because the condition that stops us from using the moving agg will be triggered here, I think we probably need to test with a bit more data...
There was a problem hiding this comment.
I made the floating point error threshold f64::INFINITY in test mode – hopefully that should do the trick here?
syvb
force-pushed
the
sv/stats_agg-twofloat
branch
from
30f00a1 to
32598c4
Compare
|
bors r+ |
|
Build succeeded:
|
Makes
stats_agguseTwoFloats internally for keeping track of the state when in moving-aggregate mode to prevent floating-point error from accumulating. See #595 for some more details.