You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "Jackie-Jiang (via GitHub)" <gi...@apache.org> on 2023/12/07 23:03:02 UTC
[PR] Fix partition handling by always using string values [pinot]
Jackie-Jiang opened a new pull request, #12115:
URL: https://github.com/apache/pinot/pull/12115
Always use string representation of the value to compute the partition so that the value from query can always match the value from the segment.
Fix the wrong handling of `BYTES` and `BIG_DECIMAL` type as partition column
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix partition handling by always using string values [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #12115:
URL: https://github.com/apache/pinot/pull/12115#discussion_r1423208731
##########
pinot-core/src/main/java/org/apache/pinot/core/segment/processing/partitioner/TableConfigPartitioner.java:
##########
@@ -41,6 +43,16 @@ public TableConfigPartitioner(String columnName, ColumnPartitionConfig columnPar
@Override
public String getPartition(GenericRow genericRow) {
- return String.valueOf(_partitionFunction.getPartition(genericRow.getValue(_column)));
+ return String.valueOf(_partitionFunction.getPartition(convertToString(genericRow.getValue(_column))));
+ }
+
+ private static String convertToString(Object value) {
Review Comment:
We can potentially check for all possible classes supported (e.g. Integer, Long, Float, Double, String etc.), but that might involve performance overhead (one if check per type). Since we don't add new data types frequently, I prefer keeping the check only for types that need to be converted
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix partition handling by always using string values [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #12115:
URL: https://github.com/apache/pinot/pull/12115#discussion_r1423332268
##########
pinot-core/src/main/java/org/apache/pinot/core/segment/processing/partitioner/TableConfigPartitioner.java:
##########
@@ -41,6 +43,16 @@ public TableConfigPartitioner(String columnName, ColumnPartitionConfig columnPar
@Override
public String getPartition(GenericRow genericRow) {
- return String.valueOf(_partitionFunction.getPartition(genericRow.getValue(_column)));
+ return String.valueOf(_partitionFunction.getPartition(convertToString(genericRow.getValue(_column))));
+ }
+
+ private static String convertToString(Object value) {
Review Comment:
Good point. Moved them into `FieldSpec` where the `DataType` is defined
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix partition handling by always using string values [pinot]
Posted by "jackjlli (via GitHub)" <gi...@apache.org>.
jackjlli commented on PR #12115:
URL: https://github.com/apache/pinot/pull/12115#issuecomment-1883677830
Hi @Jackie-Jiang @snleee @swaminathanmanish , thanks for making the code changes in this PR! We really appreciate the contribution here. While we do have some concerns on making the backward incompatible changes not just for this PR (i.e. changing the signature of a public API) but for all the future PRs, as any incompatible changes could easily impact the existing running Pinot use cases and requires admins to verify, not just for 1 cluster in 1 company but for all the Pinot clusters run in different places/companies.
If possible, can we do the following if the backward incompatibility is **inevitable** in the future PR:
* tag the PR with `incompatible` label, and at least ask for more reviewers from different companies (like StarTree, LinkedIn, Uber, etc).
* add the migration plan/action items to the PR if it's marked as backward incompatible, so that same steps can be followed by different companies to avoid anything unexpected.
By following this steps we can at least have someone to keep an eye on the incompatible changes from multiple stakeholders and have consensus promptly instead of merging it silently. Again, we really appreciate the contribution here and look forward to building the great milestones to come together! Thanks!
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix partition handling by always using string values [pinot]
Posted by "swaminathanmanish (via GitHub)" <gi...@apache.org>.
swaminathanmanish commented on code in PR #12115:
URL: https://github.com/apache/pinot/pull/12115#discussion_r1423311235
##########
pinot-core/src/main/java/org/apache/pinot/core/segment/processing/partitioner/TableConfigPartitioner.java:
##########
@@ -41,6 +43,16 @@ public TableConfigPartitioner(String columnName, ColumnPartitionConfig columnPar
@Override
public String getPartition(GenericRow genericRow) {
- return String.valueOf(_partitionFunction.getPartition(genericRow.getValue(_column)));
+ return String.valueOf(_partitionFunction.getPartition(convertToString(genericRow.getValue(_column))));
+ }
+
+ private static String convertToString(Object value) {
Review Comment:
Ok sounds good. I assume that if a new type is introduced, we anyway have to modify RequestContextUtils to add the new literal type? (where the exception is thrown). if thats the case, we can consider moving this logic and the one in MutableSegmentImpl.java to RequestContextUtils, so that they are in the same file and easy to discover all the places that needs the change.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix partition handling by always using string values [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #12115:
URL: https://github.com/apache/pinot/pull/12115
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix partition handling by always using string values [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #12115:
URL: https://github.com/apache/pinot/pull/12115#issuecomment-1922445010
@jackjlli Thanks for the suggestion! Going forward, will add a section to list all the public API changes in the PR description.
The backward incompatible changes (non API changes) are more critical because it might be hard to catch. We should always handle backward compatibility for consecutive releases, and use multiple releases to maintain backward compatibility.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix partition handling by always using string values [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #12115:
URL: https://github.com/apache/pinot/pull/12115#issuecomment-1846280186
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `36 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`e207844`)](https://app.codecov.io/gh/apache/pinot/commit/e207844038c78babab85cb73034f4238105f4468?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.64% compared to head [(`04f71c5`)](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 46.47%.
> Report is 2 commits behind head on master.
| [Files](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ot/common/request/context/RequestContextUtils.java](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L1JlcXVlc3RDb250ZXh0VXRpbHMuamF2YQ==) | 33.33% | [8 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...local/indexsegment/mutable/MutableSegmentImpl.java](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvbXV0YWJsZS9NdXRhYmxlU2VnbWVudEltcGwuamF2YQ==) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...processing/partitioner/TableConfigPartitioner.java](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvVGFibGVDb25maWdQYXJ0aXRpb25lci5qYXZh) | 33.33% | [2 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../impl/stats/AbstractColumnStatisticsCollector.java](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9zdGF0cy9BYnN0cmFjdENvbHVtblN0YXRpc3RpY3NDb2xsZWN0b3IuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../stats/BigDecimalColumnPreIndexStatsCollector.java](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9zdGF0cy9CaWdEZWNpbWFsQ29sdW1uUHJlSW5kZXhTdGF0c0NvbGxlY3Rvci5qYXZh) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...impl/stats/BytesColumnPredIndexStatsCollector.java](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9zdGF0cy9CeXRlc0NvbHVtblByZWRJbmRleFN0YXRzQ29sbGVjdG9yLmphdmE=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...impl/stats/DoubleColumnPreIndexStatsCollector.java](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9zdGF0cy9Eb3VibGVDb2x1bW5QcmVJbmRleFN0YXRzQ29sbGVjdG9yLmphdmE=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../impl/stats/FloatColumnPreIndexStatsCollector.java](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9zdGF0cy9GbG9hdENvbHVtblByZUluZGV4U3RhdHNDb2xsZWN0b3IuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...or/impl/stats/IntColumnPreIndexStatsCollector.java](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9zdGF0cy9JbnRDb2x1bW5QcmVJbmRleFN0YXRzQ29sbGVjdG9yLmphdmE=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...r/impl/stats/LongColumnPreIndexStatsCollector.java](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9zdGF0cy9Mb25nQ29sdW1uUHJlSW5kZXhTdGF0c0NvbGxlY3Rvci5qYXZh) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| ... and [1 more](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #12115 +/- ##
=============================================
- Coverage 61.64% 46.47% -15.18%
+ Complexity 1153 945 -208
=============================================
Files 2406 1808 -598
Lines 130851 95077 -35774
Branches 20218 15343 -4875
=============================================
- Hits 80665 44188 -36477
- Misses 44287 47737 +3450
+ Partials 5899 3152 -2747
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/12115/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/12115/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/12115/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/12115/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/12115/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/12115/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/12115/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.47% <48.57%> (-15.04%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/12115/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.47% <48.57%> (-15.14%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/12115/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/12115/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.47% <48.57%> (-15.18%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/12115/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.47% <48.57%> (-15.17%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/12115/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.47% <48.57%> (-0.23%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/12115/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/12115?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix partition handling by always using string values [pinot]
Posted by "swaminathanmanish (via GitHub)" <gi...@apache.org>.
swaminathanmanish commented on code in PR #12115:
URL: https://github.com/apache/pinot/pull/12115#discussion_r1423195561
##########
pinot-core/src/main/java/org/apache/pinot/core/segment/processing/partitioner/TableConfigPartitioner.java:
##########
@@ -41,6 +43,16 @@ public TableConfigPartitioner(String columnName, ColumnPartitionConfig columnPar
@Override
public String getPartition(GenericRow genericRow) {
- return String.valueOf(_partitionFunction.getPartition(genericRow.getValue(_column)));
+ return String.valueOf(_partitionFunction.getPartition(convertToString(genericRow.getValue(_column))));
+ }
+
+ private static String convertToString(Object value) {
Review Comment:
Is there a way to catch unsupported types here to ensure that for new types, we do the string conversion correctly ? Same for the logic in **MutableSegmentImpl.java** (similar to what you have in getStringValue utility)
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org