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 2021/10/17 00:32:52 UTC
[GitHub] [pinot] richardstartin opened a new pull request #7581: add `BOOLEAN_ARRAY` and `TIMESTAMP_ARRAY` types
richardstartin opened a new pull request #7581:
URL: https://github.com/apache/pinot/pull/7581
## Description
This adds MV support for `BOOLEAN` and `TIMESTAMP` fields which makes type inference from JSON safer.
## Upgrade Notes
Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR fix a zero-downtime upgrade introduced earlier?
* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR otherwise need attention when creating release notes? Things to consider:
- New configuration options
- Deprecation of configurations
- Signature changes to public methods/interfaces
- New plugins added or old plugins removed
* [ ] Yes (Please label this PR as **<code>release-notes</code>** and complete the section on Release Notes)
## Release Notes
<!-- If you have tagged this as either backward-incompat or release-notes,
you MUST add text here that you would like to see appear in release notes of the
next release. -->
<!-- If you have a series of commits adding or enabling a feature, then
add this section only in final commit that marks the feature completed.
Refer to earlier release notes to see examples of text.
-->
## Documentation
<!-- If you have introduced a new feature or configuration, please add it to the documentation as well.
See https://docs.pinot.apache.org/developers/developers-and-contributors/update-document
-->
--
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 change in pull request #7581: add `BOOLEAN_ARRAY` and `TIMESTAMP_ARRAY` types
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7581:
URL: https://github.com/apache/pinot/pull/7581#discussion_r730147912
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java
##########
@@ -434,54 +411,104 @@ public Serializable convertAndFormat(Object value) {
case INT_ARRAY:
return (int[]) value;
case LONG_ARRAY:
- if (value instanceof long[]) {
- return (long[]) value;
- } else {
- int[] intValues = (int[]) value;
- int length = intValues.length;
- long[] longValues = new long[length];
- for (int i = 0; i < length; i++) {
- longValues[i] = intValues[i];
- }
- return longValues;
- }
+ return toLongArray(value);
case FLOAT_ARRAY:
return (float[]) value;
case DOUBLE_ARRAY:
- if (value instanceof double[]) {
- return (double[]) value;
- } else if (value instanceof int[]) {
- int[] intValues = (int[]) value;
- int length = intValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = intValues[i];
- }
- return doubleValues;
- } else if (value instanceof long[]) {
- long[] longValues = (long[]) value;
- int length = longValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = longValues[i];
- }
- return doubleValues;
- } else {
- float[] floatValues = (float[]) value;
- int length = floatValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = floatValues[i];
- }
- return doubleValues;
- }
+ return toDoubleArray(value);
case STRING_ARRAY:
return (String[]) value;
+ case BOOLEAN_ARRAY:
+ return toBooleanArray(value);
+ case TIMESTAMP_ARRAY:
+ return formatTimestampArray(value);
default:
throw new IllegalStateException(String.format("Cannot convert and format: '%s' to type: %s", value, this));
}
}
+ private static double[] toDoubleArray(Object value) {
+ if (value instanceof double[]) {
+ return (double[]) value;
+ } else if (value instanceof int[]) {
+ int[] intValues = (int[]) value;
+ int length = intValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = intValues[i];
+ }
+ return doubleValues;
+ } else if (value instanceof long[]) {
+ long[] longValues = (long[]) value;
+ int length = longValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = longValues[i];
+ }
+ return doubleValues;
+ } else {
+ float[] floatValues = (float[]) value;
+ int length = floatValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = floatValues[i];
+ }
+ return doubleValues;
+ }
+ }
+
+ private static long[] toLongArray(Object value) {
+ if (value instanceof long[]) {
+ return (long[]) value;
+ } else {
+ int[] intValues = (int[]) value;
+ int length = intValues.length;
+ long[] longValues = new long[length];
+ for (int i = 0; i < length; i++) {
+ longValues[i] = intValues[i];
+ }
+ return longValues;
+ }
+ }
+
+ private static boolean[] toBooleanArray(Object value) {
+ if (value instanceof boolean[]) {
Review comment:
This `if` is not really possible because we never store `boolean[]` internal. Same for `Timestamp`
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java
##########
@@ -250,8 +251,17 @@ public int hashCode() {
LONG_ARRAY,
FLOAT_ARRAY,
DOUBLE_ARRAY,
- STRING_ARRAY;
-
+ STRING_ARRAY,
+ BOOLEAN_ARRAY /* Stored as INT_ARRAY */,
Review comment:
(nit) Consider putting them before the `STRING_ARRAY` to keep all fixed-length ones together
--
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] walterddr commented on a change in pull request #7581: add `BOOLEAN_ARRAY` and `TIMESTAMP_ARRAY` types
Posted by GitBox <gi...@apache.org>.
walterddr commented on a change in pull request #7581:
URL: https://github.com/apache/pinot/pull/7581#discussion_r729909170
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java
##########
@@ -434,54 +411,104 @@ public Serializable convertAndFormat(Object value) {
case INT_ARRAY:
return (int[]) value;
case LONG_ARRAY:
- if (value instanceof long[]) {
- return (long[]) value;
- } else {
- int[] intValues = (int[]) value;
- int length = intValues.length;
- long[] longValues = new long[length];
- for (int i = 0; i < length; i++) {
- longValues[i] = intValues[i];
- }
- return longValues;
- }
+ return toLongArray(value);
case FLOAT_ARRAY:
return (float[]) value;
case DOUBLE_ARRAY:
- if (value instanceof double[]) {
- return (double[]) value;
- } else if (value instanceof int[]) {
- int[] intValues = (int[]) value;
- int length = intValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = intValues[i];
- }
- return doubleValues;
- } else if (value instanceof long[]) {
- long[] longValues = (long[]) value;
- int length = longValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = longValues[i];
- }
- return doubleValues;
- } else {
- float[] floatValues = (float[]) value;
- int length = floatValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = floatValues[i];
- }
- return doubleValues;
- }
+ return toDoubleArray(value);
case STRING_ARRAY:
return (String[]) value;
+ case BOOLEAN_ARRAY:
+ return toBooleanArray(value);
+ case TIMESTAMP_ARRAY:
+ return formatTimestampArray(value);
default:
throw new IllegalStateException(String.format("Cannot convert and format: '%s' to type: %s", value, this));
}
}
+ private static double[] toDoubleArray(Object value) {
+ if (value instanceof double[]) {
+ return (double[]) value;
+ } else if (value instanceof int[]) {
+ int[] intValues = (int[]) value;
+ int length = intValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = intValues[i];
+ }
+ return doubleValues;
+ } else if (value instanceof long[]) {
+ long[] longValues = (long[]) value;
+ int length = longValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = longValues[i];
+ }
+ return doubleValues;
+ } else {
+ float[] floatValues = (float[]) value;
+ int length = floatValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = floatValues[i];
+ }
+ return doubleValues;
+ }
+ }
+
+ private static long[] toLongArray(Object value) {
+ if (value instanceof long[]) {
+ return (long[]) value;
+ } else {
+ int[] intValues = (int[]) value;
+ int length = intValues.length;
+ long[] longValues = new long[length];
+ for (int i = 0; i < length; i++) {
+ longValues[i] = intValues[i];
+ }
+ return longValues;
+ }
+ }
+
+ private static boolean[] toBooleanArray(Object value) {
+ if (value instanceof boolean[]) {
+ return (boolean[]) value;
+ } else {
+ int[] ints = (int[]) value;
+ boolean[] booleans = new boolean[ints.length];
+ for (int i = 0; i < ints.length; i++) {
+ booleans[i] = ints[i] == 1;
Review comment:
should this be `!= 0` ?
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java
##########
@@ -434,54 +411,104 @@ public Serializable convertAndFormat(Object value) {
case INT_ARRAY:
return (int[]) value;
case LONG_ARRAY:
- if (value instanceof long[]) {
- return (long[]) value;
- } else {
- int[] intValues = (int[]) value;
- int length = intValues.length;
- long[] longValues = new long[length];
- for (int i = 0; i < length; i++) {
- longValues[i] = intValues[i];
- }
- return longValues;
- }
+ return toLongArray(value);
case FLOAT_ARRAY:
return (float[]) value;
case DOUBLE_ARRAY:
- if (value instanceof double[]) {
- return (double[]) value;
- } else if (value instanceof int[]) {
- int[] intValues = (int[]) value;
- int length = intValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = intValues[i];
- }
- return doubleValues;
- } else if (value instanceof long[]) {
- long[] longValues = (long[]) value;
- int length = longValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = longValues[i];
- }
- return doubleValues;
- } else {
- float[] floatValues = (float[]) value;
- int length = floatValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = floatValues[i];
- }
- return doubleValues;
- }
+ return toDoubleArray(value);
case STRING_ARRAY:
return (String[]) value;
+ case BOOLEAN_ARRAY:
+ return toBooleanArray(value);
+ case TIMESTAMP_ARRAY:
+ return formatTimestampArray(value);
default:
throw new IllegalStateException(String.format("Cannot convert and format: '%s' to type: %s", value, this));
}
}
+ private static double[] toDoubleArray(Object value) {
+ if (value instanceof double[]) {
+ return (double[]) value;
+ } else if (value instanceof int[]) {
+ int[] intValues = (int[]) value;
+ int length = intValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = intValues[i];
+ }
+ return doubleValues;
+ } else if (value instanceof long[]) {
+ long[] longValues = (long[]) value;
+ int length = longValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = longValues[i];
+ }
+ return doubleValues;
+ } else {
+ float[] floatValues = (float[]) value;
+ int length = floatValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = floatValues[i];
+ }
+ return doubleValues;
+ }
+ }
+
+ private static long[] toLongArray(Object value) {
+ if (value instanceof long[]) {
+ return (long[]) value;
+ } else {
+ int[] intValues = (int[]) value;
+ int length = intValues.length;
+ long[] longValues = new long[length];
+ for (int i = 0; i < length; i++) {
+ longValues[i] = intValues[i];
+ }
+ return longValues;
+ }
+ }
+
+ private static boolean[] toBooleanArray(Object value) {
+ if (value instanceof boolean[]) {
+ return (boolean[]) value;
+ } else {
+ int[] ints = (int[]) value;
+ boolean[] booleans = new boolean[ints.length];
+ for (int i = 0; i < ints.length; i++) {
+ booleans[i] = ints[i] == 1;
Review comment:
hmm. https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/BooleanUtils.html seems to consider 0 as false and everything else as true. this is also the convention I encountered the most. but yeah if we are already doing so. it's fine
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java
##########
@@ -434,54 +411,104 @@ public Serializable convertAndFormat(Object value) {
case INT_ARRAY:
return (int[]) value;
case LONG_ARRAY:
- if (value instanceof long[]) {
- return (long[]) value;
- } else {
- int[] intValues = (int[]) value;
- int length = intValues.length;
- long[] longValues = new long[length];
- for (int i = 0; i < length; i++) {
- longValues[i] = intValues[i];
- }
- return longValues;
- }
+ return toLongArray(value);
case FLOAT_ARRAY:
return (float[]) value;
case DOUBLE_ARRAY:
- if (value instanceof double[]) {
- return (double[]) value;
- } else if (value instanceof int[]) {
- int[] intValues = (int[]) value;
- int length = intValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = intValues[i];
- }
- return doubleValues;
- } else if (value instanceof long[]) {
- long[] longValues = (long[]) value;
- int length = longValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = longValues[i];
- }
- return doubleValues;
- } else {
- float[] floatValues = (float[]) value;
- int length = floatValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = floatValues[i];
- }
- return doubleValues;
- }
+ return toDoubleArray(value);
case STRING_ARRAY:
return (String[]) value;
+ case BOOLEAN_ARRAY:
+ return toBooleanArray(value);
+ case TIMESTAMP_ARRAY:
+ return formatTimestampArray(value);
default:
throw new IllegalStateException(String.format("Cannot convert and format: '%s' to type: %s", value, this));
}
}
+ private static double[] toDoubleArray(Object value) {
+ if (value instanceof double[]) {
+ return (double[]) value;
+ } else if (value instanceof int[]) {
+ int[] intValues = (int[]) value;
+ int length = intValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = intValues[i];
+ }
+ return doubleValues;
+ } else if (value instanceof long[]) {
+ long[] longValues = (long[]) value;
+ int length = longValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = longValues[i];
+ }
+ return doubleValues;
+ } else {
+ float[] floatValues = (float[]) value;
+ int length = floatValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = floatValues[i];
+ }
+ return doubleValues;
+ }
+ }
+
+ private static long[] toLongArray(Object value) {
+ if (value instanceof long[]) {
+ return (long[]) value;
+ } else {
+ int[] intValues = (int[]) value;
+ int length = intValues.length;
+ long[] longValues = new long[length];
+ for (int i = 0; i < length; i++) {
+ longValues[i] = intValues[i];
+ }
+ return longValues;
+ }
+ }
+
+ private static boolean[] toBooleanArray(Object value) {
+ if (value instanceof boolean[]) {
+ return (boolean[]) value;
+ } else {
+ int[] ints = (int[]) value;
+ boolean[] booleans = new boolean[ints.length];
+ for (int i = 0; i < ints.length; i++) {
+ booleans[i] = ints[i] == 1;
Review comment:
hmm. https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/BooleanUtils.html seems to consider 0 as false and everything else as true. this is also the convention I encountered the most. but yeah if we are already doing so in SV. it's fine
--
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 #7581: add `BOOLEAN_ARRAY` and `TIMESTAMP_ARRAY` types
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7581:
URL: https://github.com/apache/pinot/pull/7581#issuecomment-944318970
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7581?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 [#7581](https://codecov.io/gh/apache/pinot/pull/7581?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (43cfb0e) into [master](https://codecov.io/gh/apache/pinot/commit/042119788f8af3cf22032895909245593bab0e1e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0421197) will **decrease** coverage by `56.82%`.
> The diff coverage is `0.00%`.
> :exclamation: Current head 43cfb0e differs from pull request most recent head 06db69f. Consider uploading reports for the commit 06db69f to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7581?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7581 +/- ##
=============================================
- Coverage 71.54% 14.71% -56.83%
+ Complexity 3886 80 -3806
=============================================
Files 1552 1506 -46
Lines 78974 77160 -1814
Branches 11709 11507 -202
=============================================
- Hits 56502 11355 -45147
- Misses 18670 64981 +46311
+ Partials 3802 824 -2978
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.71% <0.00%> (-0.01%)` | :arrow_down: |
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/7581?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...java/org/apache/pinot/common/utils/DataSchema.java](https://codecov.io/gh/apache/pinot/pull/7581/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%> (-75.00%)` | :arrow_down: |
| [...a/org/apache/pinot/common/utils/PinotDataType.java](https://codecov.io/gh/apache/pinot/pull/7581/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%> (-79.26%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7581/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/7581/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7581/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/7581/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/7581/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7581/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/BigDecimalUtils.java](https://codecov.io/gh/apache/pinot/pull/7581/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQmlnRGVjaW1hbFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/common/tier/TierFactory.java](https://codecov.io/gh/apache/pinot/pull/7581/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1225 more](https://codecov.io/gh/apache/pinot/pull/7581/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/7581?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/7581?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 [0421197...06db69f](https://codecov.io/gh/apache/pinot/pull/7581?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] codecov-commenter edited a comment on pull request #7581: add `BOOLEAN_ARRAY` and `TIMESTAMP_ARRAY` types
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7581:
URL: https://github.com/apache/pinot/pull/7581#issuecomment-944318970
--
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] richardstartin commented on a change in pull request #7581: add `BOOLEAN_ARRAY` and `TIMESTAMP_ARRAY` types
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #7581:
URL: https://github.com/apache/pinot/pull/7581#discussion_r730777407
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java
##########
@@ -250,8 +251,17 @@ public int hashCode() {
LONG_ARRAY,
FLOAT_ARRAY,
DOUBLE_ARRAY,
- STRING_ARRAY;
-
+ STRING_ARRAY,
+ BOOLEAN_ARRAY /* Stored as INT_ARRAY */,
Review comment:
ok - I can do this, but have a reflex for adding enum values at the end so the ordinals are stable.
--
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] kishoreg commented on pull request #7581: add `BOOLEAN_ARRAY` and `TIMESTAMP_ARRAY` types
Posted by GitBox <gi...@apache.org>.
kishoreg commented on pull request #7581:
URL: https://github.com/apache/pinot/pull/7581#issuecomment-944348377
Thanks for adding this. LGTM
--
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] richardstartin commented on a change in pull request #7581: add `BOOLEAN_ARRAY` and `TIMESTAMP_ARRAY` types
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #7581:
URL: https://github.com/apache/pinot/pull/7581#discussion_r729801948
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java
##########
@@ -250,8 +251,17 @@ public int hashCode() {
LONG_ARRAY,
FLOAT_ARRAY,
DOUBLE_ARRAY,
- STRING_ARRAY;
-
+ STRING_ARRAY,
+ BOOLEAN_ARRAY /* Stored as INT_ARRAY */,
+ TIMESTAMP_ARRAY /* Stored as LONG_ARRAY */;
+
+ private static final EnumSet<ColumnDataType> NUMERIC_TYPES = EnumSet.of(INT, LONG, FLOAT, DOUBLE);
+ private static final EnumSet<ColumnDataType> INTEGRAL_TYPES = EnumSet.of(INT, LONG);
+ private static final EnumSet<ColumnDataType> ARRAY_TYPES = EnumSet.of(INT_ARRAY, LONG_ARRAY, FLOAT_ARRAY,
+ DOUBLE_ARRAY, STRING_ARRAY, BOOLEAN_ARRAY, TIMESTAMP_ARRAY);
+ private static final EnumSet<ColumnDataType> NUMERIC_ARRAY_TYPES = EnumSet.of(INT_ARRAY, LONG_ARRAY, FLOAT_ARRAY,
+ DOUBLE_ARRAY);
+ private static final EnumSet<ColumnDataType> INTEGRAL_ARRAY_TYPES = EnumSet.of(INT_ARRAY, LONG_ARRAY);
Review comment:
I took the opportunity to clean this up a little bit since I needed to add to what didn't look like very scalable code readability wise. I have no reason to believe this code is performance-critical but lookups into a constant `EnumSet` will perform well.
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java
##########
@@ -414,74 +391,98 @@ public Serializable format(Object value) {
*/
public Serializable convertAndFormat(Object value) {
switch (this) {
+ case TIMESTAMP:
Review comment:
I removed a lot of duplication here so it's easier to follow which types need special formatting and which don't.
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java
##########
@@ -414,74 +391,98 @@ public Serializable format(Object value) {
*/
public Serializable convertAndFormat(Object value) {
switch (this) {
+ case TIMESTAMP:
Review comment:
I removed a lot of duplication here so it's easier to follow which types need special formatting and which don't.
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java
##########
@@ -434,54 +411,104 @@ public Serializable convertAndFormat(Object value) {
case INT_ARRAY:
return (int[]) value;
case LONG_ARRAY:
- if (value instanceof long[]) {
- return (long[]) value;
- } else {
- int[] intValues = (int[]) value;
- int length = intValues.length;
- long[] longValues = new long[length];
- for (int i = 0; i < length; i++) {
- longValues[i] = intValues[i];
- }
- return longValues;
- }
+ return toLongArray(value);
case FLOAT_ARRAY:
return (float[]) value;
case DOUBLE_ARRAY:
- if (value instanceof double[]) {
- return (double[]) value;
- } else if (value instanceof int[]) {
- int[] intValues = (int[]) value;
- int length = intValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = intValues[i];
- }
- return doubleValues;
- } else if (value instanceof long[]) {
- long[] longValues = (long[]) value;
- int length = longValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = longValues[i];
- }
- return doubleValues;
- } else {
- float[] floatValues = (float[]) value;
- int length = floatValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = floatValues[i];
- }
- return doubleValues;
- }
+ return toDoubleArray(value);
case STRING_ARRAY:
return (String[]) value;
+ case BOOLEAN_ARRAY:
+ return toBooleanArray(value);
+ case TIMESTAMP_ARRAY:
+ return formatTimestampArray(value);
default:
throw new IllegalStateException(String.format("Cannot convert and format: '%s' to type: %s", value, this));
}
}
+ private static double[] toDoubleArray(Object value) {
+ if (value instanceof double[]) {
+ return (double[]) value;
+ } else if (value instanceof int[]) {
+ int[] intValues = (int[]) value;
+ int length = intValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = intValues[i];
+ }
+ return doubleValues;
+ } else if (value instanceof long[]) {
+ long[] longValues = (long[]) value;
+ int length = longValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = longValues[i];
+ }
+ return doubleValues;
+ } else {
+ float[] floatValues = (float[]) value;
+ int length = floatValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = floatValues[i];
+ }
+ return doubleValues;
+ }
+ }
+
+ private static long[] toLongArray(Object value) {
+ if (value instanceof long[]) {
+ return (long[]) value;
+ } else {
+ int[] intValues = (int[]) value;
+ int length = intValues.length;
+ long[] longValues = new long[length];
+ for (int i = 0; i < length; i++) {
+ longValues[i] = intValues[i];
+ }
+ return longValues;
+ }
+ }
+
+ private static boolean[] toBooleanArray(Object value) {
+ if (value instanceof boolean[]) {
+ return (boolean[]) value;
+ } else {
+ int[] ints = (int[]) value;
+ boolean[] booleans = new boolean[ints.length];
+ for (int i = 0; i < ints.length; i++) {
+ booleans[i] = ints[i] == 1;
Review comment:
no, see the SV equivalent:
```java
case BOOLEAN:
return (Integer) value == 1;
```
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java
##########
@@ -434,54 +411,104 @@ public Serializable convertAndFormat(Object value) {
case INT_ARRAY:
return (int[]) value;
case LONG_ARRAY:
- if (value instanceof long[]) {
- return (long[]) value;
- } else {
- int[] intValues = (int[]) value;
- int length = intValues.length;
- long[] longValues = new long[length];
- for (int i = 0; i < length; i++) {
- longValues[i] = intValues[i];
- }
- return longValues;
- }
+ return toLongArray(value);
case FLOAT_ARRAY:
return (float[]) value;
case DOUBLE_ARRAY:
- if (value instanceof double[]) {
- return (double[]) value;
- } else if (value instanceof int[]) {
- int[] intValues = (int[]) value;
- int length = intValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = intValues[i];
- }
- return doubleValues;
- } else if (value instanceof long[]) {
- long[] longValues = (long[]) value;
- int length = longValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = longValues[i];
- }
- return doubleValues;
- } else {
- float[] floatValues = (float[]) value;
- int length = floatValues.length;
- double[] doubleValues = new double[length];
- for (int i = 0; i < length; i++) {
- doubleValues[i] = floatValues[i];
- }
- return doubleValues;
- }
+ return toDoubleArray(value);
case STRING_ARRAY:
return (String[]) value;
+ case BOOLEAN_ARRAY:
+ return toBooleanArray(value);
+ case TIMESTAMP_ARRAY:
+ return formatTimestampArray(value);
default:
throw new IllegalStateException(String.format("Cannot convert and format: '%s' to type: %s", value, this));
}
}
+ private static double[] toDoubleArray(Object value) {
+ if (value instanceof double[]) {
+ return (double[]) value;
+ } else if (value instanceof int[]) {
+ int[] intValues = (int[]) value;
+ int length = intValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = intValues[i];
+ }
+ return doubleValues;
+ } else if (value instanceof long[]) {
+ long[] longValues = (long[]) value;
+ int length = longValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = longValues[i];
+ }
+ return doubleValues;
+ } else {
+ float[] floatValues = (float[]) value;
+ int length = floatValues.length;
+ double[] doubleValues = new double[length];
+ for (int i = 0; i < length; i++) {
+ doubleValues[i] = floatValues[i];
+ }
+ return doubleValues;
+ }
+ }
+
+ private static long[] toLongArray(Object value) {
+ if (value instanceof long[]) {
+ return (long[]) value;
+ } else {
+ int[] intValues = (int[]) value;
+ int length = intValues.length;
+ long[] longValues = new long[length];
+ for (int i = 0; i < length; i++) {
+ longValues[i] = intValues[i];
+ }
+ return longValues;
+ }
+ }
+
+ private static boolean[] toBooleanArray(Object value) {
+ if (value instanceof boolean[]) {
+ return (boolean[]) value;
+ } else {
+ int[] ints = (int[]) value;
+ boolean[] booleans = new boolean[ints.length];
+ for (int i = 0; i < ints.length; i++) {
+ booleans[i] = ints[i] == 1;
Review comment:
Yes, this is for mapping to truth values from the way booleans are physically stored in pinot, and isn't a generic 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
[GitHub] [pinot] richardstartin commented on pull request #7581: add `BOOLEAN_ARRAY` and `TIMESTAMP_ARRAY` types
Posted by GitBox <gi...@apache.org>.
richardstartin commented on pull request #7581:
URL: https://github.com/apache/pinot/pull/7581#issuecomment-944373378
It looks like Pinot Quickstart JDK1.8 is stuck in a loop https://github.com/apache/pinot/pull/7581/checks?check_run_id=3906427468
--
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 merged pull request #7581: add `BOOLEAN_ARRAY` and `TIMESTAMP_ARRAY` types
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #7581:
URL: https://github.com/apache/pinot/pull/7581
--
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