You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/04/08 08:56:46 UTC
[GitHub] [pinot] nizarhejazi opened a new pull request, #8468: Support Single-valued BigDecimal columns across different SQL statements, and Pinot transforms and operators
nizarhejazi opened a new pull request, #8468:
URL: https://github.com/apache/pinot/pull/8468
## Description
Add support for single-valued `BigDecimal` data type (`BIG_DECIMAL`) across different SQL statements and Pinot functions. The underlying stored data type is `byte[]`.
- Conversion between different Pinot data types and BigDecimal.
- Document the reason for choosing specific BigDecimal methods (constructor vs. valueOf, equals vs. compareTo, etc.) and accompanying data structures (TreeSet vs. HashSet, ObjectMapper with ExactBigDecimals etc.)
- Extend DataFetcher to fetch BigDecimal values.
- Support BIG_DECIMAL in:
- SELECT statement.
- DISTINCT statement.
- ORDER BY statement.
- GROUP BY statement.
- HAVING statement.
- CASE statement.
- CAST transform.
- Groovy transform.
- Identifier transform.
- Literal transform.
- Lookup transform.
- JsonPathEvaluator (w/ custom ObjectMapper and JsonNodeFactory).
- JsonExtractScalar transform.
- Addition, Subtraction, Multiplication, and Division.
- abs(), ceil(), and floor() math functions.
- Greatest, and Least transforms.
- EQUAL, NOT_EQUAL, IN, NOT_IN, RANGE predicates.
- NoDictionary based EQUAL, NOT_EQUAL and IN predicates.
- Binary operators (=, !=, >=, >, <=, <).
- BigDecimalDictionary
- BaseDataTable.
- FieldSpec.
- ~27 unit tests.
## Release Notes
**<code>release-notes</code>**
--
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
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #8468: Support Single-valued BigDecimal columns across different SQL statements, and Pinot transforms and operators
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #8468:
URL: https://github.com/apache/pinot/pull/8468#discussion_r845649173
##########
pinot-core/src/main/java/org/apache/pinot/core/common/BlockValSet.java:
##########
@@ -84,6 +85,13 @@
*/
double[] getDoubleValuesSV();
+ /**
+ * Returns the BigDecimal[] values for a single-valued column.
+ *
+ * @return Array of BigDecimal[] values
+ */
+ BigDecimal[] getBigDecimalValuesSV();
Review Comment:
I don't follow. We should just use `getBytesValuesSV` for `BigDecimal` types
--
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
[GitHub] [pinot] KKcorps commented on pull request #8468: Support Single-valued BigDecimal columns across different SQL statements, and Pinto transforms and operators
Posted by GitBox <gi...@apache.org>.
KKcorps commented on PR #8468:
URL: https://github.com/apache/pinot/pull/8468#issuecomment-1088740199
Not really sure if we need to implement `transformToBigDecimalValuesSV` in all the functions. We can use `transformToBytesValuesSV` I think
--
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
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #8468: Support Single-valued BigDecimal columns across different SQL statements, and Pinot transforms and operators
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #8468:
URL: https://github.com/apache/pinot/pull/8468#discussion_r844346048
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/PinotDataType.java:
##########
@@ -136,6 +143,11 @@ public double toDouble(Object value) {
return ((Number) value).doubleValue();
}
+ @Override
+ public BigDecimal toBigDecimal(Object value) {
+ return new BigDecimal(toInt(value));
Review Comment:
I think it should be the same because internally the value is casted to `long`. For small int, we can reuse the object
--
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
[GitHub] [pinot] nizarhejazi commented on a diff in pull request #8468: Support Single-valued BigDecimal columns across different SQL statements, and Pinot transforms and operators
Posted by GitBox <gi...@apache.org>.
nizarhejazi commented on code in PR #8468:
URL: https://github.com/apache/pinot/pull/8468#discussion_r844294458
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/PinotDataType.java:
##########
@@ -136,6 +143,11 @@ public double toDouble(Object value) {
return ((Number) value).doubleValue();
}
+ @Override
+ public BigDecimal toBigDecimal(Object value) {
+ return new BigDecimal(toInt(value));
Review Comment:
Using `BigDecimal.valueOf()` requires extra cast from `int` to `long` and hence I use the constructor that takes int.
--
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
[GitHub] [pinot] codecov-commenter commented on pull request #8468: Support Single-valued BigDecimal columns across different SQL statements, and Pinot transforms and operators
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #8468:
URL: https://github.com/apache/pinot/pull/8468#issuecomment-1089400722
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8468?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8468](https://codecov.io/gh/apache/pinot/pull/8468?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ed10018) into [master](https://codecov.io/gh/apache/pinot/commit/2d22c6d12ceab2e50de0db72510de47fb2c6ae48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2d22c6d) will **decrease** coverage by `49.53%`.
> The diff coverage is `0.00%`.
> :exclamation: Current head ed10018 differs from pull request most recent head 7475694. Consider uploading reports for the commit 7475694 to get more accurate results
```diff
@@ Coverage Diff @@
## master #8468 +/- ##
=============================================
- Coverage 63.56% 14.03% -49.54%
+ Complexity 4198 84 -4114
=============================================
Files 1652 1622 -30
Lines 86988 86281 -707
Branches 13189 13183 -6
=============================================
- Hits 55298 12108 -43190
- Misses 27645 73255 +45610
+ Partials 4045 918 -3127
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.03% <0.00%> (?)` | |
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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8468?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...rg/apache/pinot/common/function/FunctionUtils.java](https://codecov.io/gh/apache/pinot/pull/8468/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRnVuY3Rpb25VdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/common/utils/DataSchema.java](https://codecov.io/gh/apache/pinot/pull/8468/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvRGF0YVNjaGVtYS5qYXZh) | `0.00% <0.00%> (-79.33%)` | :arrow_down: |
| [.../java/org/apache/pinot/common/utils/DataTable.java](https://codecov.io/gh/apache/pinot/pull/8468/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvRGF0YVRhYmxlLmphdmE=) | `0.00% <ø> (-89.48%)` | :arrow_down: |
| [...a/org/apache/pinot/common/utils/PinotDataType.java](https://codecov.io/gh/apache/pinot/pull/8468/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvUGlub3REYXRhVHlwZS5qYXZh) | `0.00% <0.00%> (-81.07%)` | :arrow_down: |
| [...a/org/apache/pinot/core/common/DataBlockCache.java](https://codecov.io/gh/apache/pinot/pull/8468/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vRGF0YUJsb2NrQ2FjaGUuamF2YQ==) | `0.00% <0.00%> (-91.49%)` | :arrow_down: |
| [...java/org/apache/pinot/core/common/DataFetcher.java](https://codecov.io/gh/apache/pinot/pull/8468/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vRGF0YUZldGNoZXIuamF2YQ==) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [...e/pinot/core/common/RowBasedBlockValueFetcher.java](https://codecov.io/gh/apache/pinot/pull/8468/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vUm93QmFzZWRCbG9ja1ZhbHVlRmV0Y2hlci5qYXZh) | `0.00% <0.00%> (-94.67%)` | :arrow_down: |
| [...che/pinot/core/common/datatable/BaseDataTable.java](https://codecov.io/gh/apache/pinot/pull/8468/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vZGF0YXRhYmxlL0Jhc2VEYXRhVGFibGUuamF2YQ==) | `0.00% <0.00%> (-81.68%)` | :arrow_down: |
| [...re/common/evaluators/DefaultJsonPathEvaluator.java](https://codecov.io/gh/apache/pinot/pull/8468/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vZXZhbHVhdG9ycy9EZWZhdWx0SnNvblBhdGhFdmFsdWF0b3IuamF2YQ==) | `0.00% <0.00%> (-31.44%)` | :arrow_down: |
| [...core/operator/blocks/IntermediateResultsBlock.java](https://codecov.io/gh/apache/pinot/pull/8468/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvSW50ZXJtZWRpYXRlUmVzdWx0c0Jsb2NrLmphdmE=) | `0.00% <0.00%> (-82.99%)` | :arrow_down: |
| ... and [1584 more](https://codecov.io/gh/apache/pinot/pull/8468/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8468?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8468?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [2d22c6d...7475694](https://codecov.io/gh/apache/pinot/pull/8468?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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
[GitHub] [pinot] nizarhejazi commented on a diff in pull request #8468: Support Single-valued BigDecimal columns across different SQL statements, and Pinot transforms and operators
Posted by GitBox <gi...@apache.org>.
nizarhejazi commented on code in PR #8468:
URL: https://github.com/apache/pinot/pull/8468#discussion_r844303267
##########
pinot-core/src/main/java/org/apache/pinot/core/common/BlockValSet.java:
##########
@@ -84,6 +85,13 @@
*/
double[] getDoubleValuesSV();
+ /**
+ * Returns the BigDecimal[] values for a single-valued column.
+ *
+ * @return Array of BigDecimal[] values
+ */
+ BigDecimal[] getBigDecimalValuesSV();
Review Comment:
@Jackie-Jiang Added it for convenience to avoid calling two APIs: `get[Type]ValuesSV` and `ArrayCopyUtils.copy` everywhere for every [Type].
--
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
[GitHub] [pinot] nizarhejazi commented on a diff in pull request #8468: Support Single-valued BigDecimal columns across different SQL statements, and Pinot transforms and operators
Posted by GitBox <gi...@apache.org>.
nizarhejazi commented on code in PR #8468:
URL: https://github.com/apache/pinot/pull/8468#discussion_r844303267
##########
pinot-core/src/main/java/org/apache/pinot/core/common/BlockValSet.java:
##########
@@ -84,6 +85,13 @@
*/
double[] getDoubleValuesSV();
+ /**
+ * Returns the BigDecimal[] values for a single-valued column.
+ *
+ * @return Array of BigDecimal[] values
+ */
+ BigDecimal[] getBigDecimalValuesSV();
Review Comment:
@Jackie-Jiang Added it for convenience to avoid calling two APIs: `getBytesValuesSV` and `ArrayCopyUtils.copy`.
I will address and move the logic to IdentifierTransformFunction.
--
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
[GitHub] [pinot] nizarhejazi commented on pull request #8468: Support Single-valued BigDecimal columns across different SQL statements, and Pinot transforms and operators
Posted by GitBox <gi...@apache.org>.
nizarhejazi commented on PR #8468:
URL: https://github.com/apache/pinot/pull/8468#issuecomment-1092618283
Agreed w/ @Jackie-Jiang to split the PR into multiple PRs.
--
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
[GitHub] [pinot] nizarhejazi commented on pull request #8468: Support Single-valued BigDecimal columns across different SQL statements, and Pinot transforms and operators
Posted by GitBox <gi...@apache.org>.
nizarhejazi commented on PR #8468:
URL: https://github.com/apache/pinot/pull/8468#issuecomment-1089563906
> Not really sure if we need to implement `transformToBigDecimalValuesSV` in all the functions. We can use `transformToBytesValuesSV` I think
- Operations on BigDecimals are in most cases not equivalent to the same operation on the byte[] representation of these big decimals.
- We need to use Presto on top of Pinot and we cannot apply BigDecimalUtils.deserialize/serialize everywhere we work with Decimals. We generate Presto SQL and this gets translated into Pinot.
- There are places (not visible in the SQL query) where dealing with BigDecimals is needed for producing correct results.
--
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
[GitHub] [pinot] nizarhejazi closed pull request #8468: Support Single-valued BigDecimal columns across different SQL statements, and Pinot transforms and operators
Posted by GitBox <gi...@apache.org>.
nizarhejazi closed pull request #8468: Support Single-valued BigDecimal columns across different SQL statements, and Pinot transforms and operators
URL: https://github.com/apache/pinot/pull/8468
--
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
[GitHub] [pinot] nizarhejazi commented on a diff in pull request #8468: Support Single-valued BigDecimal columns across different SQL statements, and Pinot transforms and operators
Posted by GitBox <gi...@apache.org>.
nizarhejazi commented on code in PR #8468:
URL: https://github.com/apache/pinot/pull/8468#discussion_r844303267
##########
pinot-core/src/main/java/org/apache/pinot/core/common/BlockValSet.java:
##########
@@ -84,6 +85,13 @@
*/
double[] getDoubleValuesSV();
+ /**
+ * Returns the BigDecimal[] values for a single-valued column.
+ *
+ * @return Array of BigDecimal[] values
+ */
+ BigDecimal[] getBigDecimalValuesSV();
Review Comment:
@Jackie-Jiang Added it for convenience to avoid calling two APIs: `getBytesValuesSV` and `ArrayCopyUtils.copy` everywhere.
Will remove and move the logic higher up to different transforms.
--
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
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #8468: Support Single-valued BigDecimal columns across different SQL statements, and Pinot transforms and operators
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #8468:
URL: https://github.com/apache/pinot/pull/8468#discussion_r844235906
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java:
##########
@@ -250,6 +251,7 @@ public int hashCode() {
FLOAT,
DOUBLE,
BOOLEAN /* Stored as INT */,
+ BIG_DECIMAL /* Stored as BYTES */,
Review Comment:
(minor) Move it in front of BOOLEAN for consistency (Also keeping number types together)
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionUtils.java:
##########
@@ -42,6 +43,7 @@ private FunctionUtils() {
put(double.class, PinotDataType.DOUBLE);
put(Double.class, PinotDataType.DOUBLE);
put(boolean.class, PinotDataType.BOOLEAN);
+ put(BigDecimal.class, PinotDataType.BIG_DECIMAL);
Review Comment:
(minor) Move it in front of boolean for consistency
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/DataTable.java:
##########
@@ -56,6 +57,8 @@
double getDouble(int rowId, int colId);
+ BigDecimal getBigDecimal(int rowId, int colId);
Review Comment:
For data access layer, we should not introduce the `BigDecimal` type, but use the stored type (see how `Timestamp` is handled)
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/PinotDataType.java:
##########
@@ -136,6 +143,11 @@ public double toDouble(Object value) {
return ((Number) value).doubleValue();
}
+ @Override
+ public BigDecimal toBigDecimal(Object value) {
+ return new BigDecimal(toInt(value));
Review Comment:
Should we consider using `BigDecimal.valueOf()`? Same for other places
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/PinotDataType.java:
##########
@@ -1281,16 +1419,20 @@ public static PinotDataType getPinotDataTypeForIngestion(FieldSpec fieldSpec) {
return fieldSpec.isSingleValueField() ? FLOAT : FLOAT_ARRAY;
case DOUBLE:
return fieldSpec.isSingleValueField() ? DOUBLE : DOUBLE_ARRAY;
+ case BIG_DECIMAL:
+ if (fieldSpec.isSingleValueField()) {
+ return BIG_DECIMAL;
+ }
+ throw new UnsupportedOperationException("Multi-value BigDecimal data type is not currently supported");
Review Comment:
(minor) Let's keep the exception type consistent
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/PinotDataType.java:
##########
@@ -1192,6 +1327,9 @@ public static PinotDataType getSingleValueType(Class<?> cls) {
if (cls == String.class) {
return STRING;
}
+ if (cls == BigDecimal.class) {
Review Comment:
(minor) Move it after `Timestamp` as it should be rarely used as the input object
##########
pinot-core/src/main/java/org/apache/pinot/core/common/BlockValSet.java:
##########
@@ -84,6 +85,13 @@
*/
double[] getDoubleValuesSV();
+ /**
+ * Returns the BigDecimal[] values for a single-valued column.
+ *
+ * @return Array of BigDecimal[] values
+ */
+ BigDecimal[] getBigDecimalValuesSV();
Review Comment:
This API is not required as we don't want to make `BigDecimal` as a storage type
--
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