-
Notifications
You must be signed in to change notification settings - Fork 443
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: adhere write stats configuration #3209
base: main
Are you sure you want to change the base?
Conversation
317766f
to
53696e2
Compare
53696e2
to
5b26be3
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3209 +/- ##
==========================================
+ Coverage 72.10% 72.20% +0.09%
==========================================
Files 138 138
Lines 45320 45569 +249
Branches 45320 45569 +249
==========================================
+ Hits 32678 32903 +225
- Misses 10567 10596 +29
+ Partials 2075 2070 -5 ☔ View full report in Codecov by Sentry. |
@ion-elgreco one question :). We can certainly merge this fix the way it is implemented now, but ... delta-kernel-rs contains some nice facilities to transform schemas in a more structured manner. In some cases implementations can be very simple. While a little more work, this could also brings us forward in adopting kernel etc and simplifying our codebase :). Using this can of course also be deferred to a later PR. |
@roeap I can look into that later for sure in a separate PR, but let's merge this to solve some current inconsistenies |
Signed-off-by: Ion Koutsouris <[email protected]>
5b26be3
to
e866db8
Compare
@roeap can we merge this? |
Description
@roeap according to the protocol we should only write struct stats when enabled. Currently we were ignoring this config and always writing struct stats. The table config is now adhered when either of two are disabled.
delta.checkpoint.writeStatsAsStruct
#3207