Allow String / numeric data type for deleteRecordColumn config #12222
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #12222 +/- ##
============================================
+ Coverage 61.57% 61.59% +0.02%
- Complexity 1152 1153 +1
============================================
Files 2415 2416 +1
Lines 131167 131205 +38
Branches 20245 20250 +5
============================================
+ Hits 80769 80821 +52
+ Misses 44501 44473 -28
- Partials 5897 5911 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Hey @walterddr @navina can you help review this PR? Should we remove boolean check altogether or add String / Numeric as additional checks? |
|
@navina please share your thoughts. i am ok with the approach as long as we agree upon the new contract |
i think it will be better to add additional string/numeric checks in TableConfigUtils. Do you think that is possible? Apologies for the delay in response @tibrewalpratik17 . I missed the notification. |
Yes let me do that instead. |
tibrewalpratik17
changed the title
tibrewalpratik17
changed the title
|
Updated! cc @walterddr @navina |
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
Outdated
Show resolved
Hide resolved
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
Outdated
Show resolved
Hide resolved
…e#12222) * Remove enforcement of BOOLEAN data type from deleteRecordColumn * add string and numeric type for boolean columns * addressed comments * improve error messages
Relates to #12217.
This patch add the criteria that
deleteRecordColumnfield can be of String / numeric data type thus allowing String based "true"/"false"/"0"/"1" to work or int based 0/1 as well based on BooleanUtils class.Tested in one of our clusters using String based field for deleted column and worked as expected.