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/02/28 20:40:58 UTC
[GitHub] [pinot] KKcorps opened a new pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
KKcorps opened a new pull request #8264:
URL: https://github.com/apache/pinot/pull/8264
This PR addresses the issue #8251 . Currently, Pinot supports `IS NULL` syntax in filter expressions such as `WHERE` clause. But it doesn't support as part of SELECT expressions such as `CASE WHEN col IS NULL THEN a ELSE b`
The PR adds support for such expressions by utilising the `NullValueVectorReader` to get the docIds which are null.
The implementation has two requirements/limitations -
* `nullHandlingEnabled` should be set to `true` since native NULL values are not supported in Pinot and requires a null value bitmap index.
* Only column names can be provided as the first argument for expressions. The support for functions is still not there.
e.g. `colA IS NULL` is supported but `LOWER(colA) IS NULL` is not supported.
Currently Pending -
* Unit Tests
* Documentation
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#issuecomment-1071747264
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8264?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 [#8264](https://codecov.io/gh/apache/pinot/pull/8264?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7cd61c1) into [master](https://codecov.io/gh/apache/pinot/commit/25cd6e34b7a6b1b013e7181bfe466677cdc0a61d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (25cd6e3) will **decrease** coverage by `55.54%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #8264 +/- ##
=============================================
- Coverage 69.72% 14.18% -55.55%
+ Complexity 4264 81 -4183
=============================================
Files 1639 1596 -43
Lines 85920 84103 -1817
Branches 12922 12733 -189
=============================================
- Hits 59906 11927 -47979
- Misses 21842 71284 +49442
+ Partials 4172 892 -3280
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.18% <0.00%> (-0.02%)` | :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/8264?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ransform/function/IdentifierTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSWRlbnRpZmllclRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `0.00% <0.00%> (-72.10%)` | :arrow_down: |
| [...transform/function/IsNotNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOb3ROdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...or/transform/function/IsNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `0.00% <0.00%> (-83.64%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/8264/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/8264/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/8264/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/8264/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/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1286 more](https://codecov.io/gh/apache/pinot/pull/8264/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/8264?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/8264?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 [25cd6e3...7cd61c1](https://codecov.io/gh/apache/pinot/pull/8264?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] KKcorps commented on a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r831442675
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNullTransformFunction.java
##########
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+ private PeekableIntIterator _nullValueVectorIterator;
+ private NullValueVectorReader _nullValueVectorReader;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1, "Exact 1 argument is required for IS_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ _nullValueVectorReader = _dataSourceMap.get(columnName).getNullValueVector();
+ if (_nullValueVectorReader != null) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ } else {
+ _nullValueVectorIterator = null;
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ _results = new int[length];
Review comment:
done
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r816239031
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,69 @@
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NOT_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1,
+ "Exact 1 argument is required for IS_NOT_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NOT_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ if (_results == null || _results.length < length) {
+ _results = new int[length];
+ }
+
+ int[] docIds = projectionBlock.getDocIds();
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ NullValueVectorReader nullValueVector = _dataSourceMap.get(columnName).getNullValueVector();
+
+ if (nullValueVector != null) {
+ for (int idx = 0; idx < length; idx++) {
+ int docId = docIds[idx];
+ if (nullValueVector.isNull(docId)) {
+ _results[idx] = 0;
+ } else {
+ _results[idx] = 1;
+ }
+ }
Review comment:
Same below - the reason is the complexity is O(n * log (m)) where n is the number of rows and m is the number of null values, but it could be O(n) (fill the array with ones then iterate over the null value bitmap).
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#issuecomment-1071747264
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8264?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 [#8264](https://codecov.io/gh/apache/pinot/pull/8264?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8449ece) into [master](https://codecov.io/gh/apache/pinot/commit/e6330bb7297c2d6f80657e4db8619ed34ccfda43?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e6330bb) will **decrease** coverage by `0.00%`.
> The diff coverage is `55.16%`.
> :exclamation: Current head 8449ece differs from pull request most recent head 983a336. Consider uploading reports for the commit 983a336 to get more accurate results
```diff
@@ Coverage Diff @@
## master #8264 +/- ##
============================================
- Coverage 64.06% 64.05% -0.01%
- Complexity 4246 4265 +19
============================================
Files 1586 1596 +10
Lines 83399 84111 +712
Branches 12641 12732 +91
============================================
+ Hits 53427 53876 +449
- Misses 26129 26343 +214
- Partials 3843 3892 +49
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests1 | `66.97% <57.40%> (-0.05%)` | :arrow_down: |
| unittests2 | `14.18% <19.71%> (+0.09%)` | :arrow_up: |
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/8264?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...roker/requesthandler/GrpcBrokerRequestHandler.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvR3JwY0Jyb2tlclJlcXVlc3RIYW5kbGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...va/org/apache/pinot/common/config/NettyConfig.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL05ldHR5Q29uZmlnLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...pache/pinot/common/config/provider/TableCache.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL3Byb3ZpZGVyL1RhYmxlQ2FjaGUuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...apache/pinot/common/helix/ExtraInstanceConfig.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vaGVsaXgvRXh0cmFJbnN0YW5jZUNvbmZpZy5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...g/apache/pinot/common/metrics/ControllerGauge.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyR2F1Z2UuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...g/apache/pinot/common/utils/helix/HelixHelper.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvaGVsaXgvSGVsaXhIZWxwZXIuamF2YQ==) | `13.30% <0.00%> (-2.72%)` | :arrow_down: |
| [.../org/apache/pinot/core/common/MinionConstants.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vTWluaW9uQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (ø)` | |
| [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `50.00% <ø> (ø)` | |
| [...ata/manager/realtime/RealtimeTableDataManager.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVUYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `11.64% <ø> (ø)` | |
| [.../pinot/core/operator/filter/NotFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvTm90RmlsdGVyT3BlcmF0b3IuamF2YQ==) | `66.66% <ø> (+11.11%)` | :arrow_up: |
| ... and [174 more](https://codecov.io/gh/apache/pinot/pull/8264/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/8264?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/8264?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 [e6330bb...983a336](https://codecov.io/gh/apache/pinot/pull/8264?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] Jackie-Jiang commented on a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r830255356
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NOT_NULL";
Review comment:
We usually use `TransformFunctionType.getName()` as the function name for transform function
##########
File path: pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
##########
@@ -610,6 +610,10 @@ private static Expression toExpression(SqlNode node) {
return RequestUtils.getIdentifierExpression(node.toString());
case LITERAL:
return RequestUtils.getLiteralExpression((SqlLiteral) node);
+ case IS_NULL:
Review comment:
The change in this class is not required as they can be handled the same as regular function (e.g. no special handling for `GREATER_THAN`)
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NOT_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+ private PeekableIntIterator _nullValueVectorIterator;
+ private NullValueVectorReader _nullValueVectorReader;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1,
+ "Exact 1 argument is required for IS_NOT_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NOT_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ _nullValueVectorReader = _dataSourceMap.get(columnName).getNullValueVector();
+ if (_nullValueVectorReader != null) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ } else {
+ _nullValueVectorIterator = null;
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ if (_results == null || _results.length < length) {
+ _results = new int[length];
+ }
+
+ int[] docIds = projectionBlock.getDocIds();
+
+ if (!_nullValueVectorIterator.hasNext() || (length > 0 && _nullValueVectorIterator.peekNext() > docIds[0])) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ }
Review comment:
This part is redundant. The docIds passed in should have no overlap with the previous array.
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
Review comment:
Suggest extracting the common part for these 2 functions. `init()` and `getResultMetadata()` can be shared.
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNullTransformFunction.java
##########
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+ private PeekableIntIterator _nullValueVectorIterator;
+ private NullValueVectorReader _nullValueVectorReader;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1, "Exact 1 argument is required for IS_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ _nullValueVectorReader = _dataSourceMap.get(columnName).getNullValueVector();
+ if (_nullValueVectorReader != null) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ } else {
+ _nullValueVectorIterator = null;
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ _results = new int[length];
Review comment:
+1
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NOT_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+ private PeekableIntIterator _nullValueVectorIterator;
+ private NullValueVectorReader _nullValueVectorReader;
Review comment:
`_leftTransformFunction`, `_dataSourceMap` and `_nullValueVectorReader` can be changed to local variable. Also, suggest renaming `_leftTransformFunction` as there is only one single argument
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#issuecomment-1071747264
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8264?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 [#8264](https://codecov.io/gh/apache/pinot/pull/8264?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d7de16f) into [master](https://codecov.io/gh/apache/pinot/commit/25cd6e34b7a6b1b013e7181bfe466677cdc0a61d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (25cd6e3) will **decrease** coverage by `40.81%`.
> The diff coverage is `8.82%`.
```diff
@@ Coverage Diff @@
## master #8264 +/- ##
=============================================
- Coverage 69.72% 28.90% -40.82%
=============================================
Files 1639 1629 -10
Lines 85920 85630 -290
Branches 12922 12898 -24
=============================================
- Hits 59906 24753 -35153
- Misses 21842 58591 +36749
+ Partials 4172 2286 -1886
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `28.90% <8.82%> (+0.03%)` | :arrow_up: |
| unittests1 | `?` | |
| unittests2 | `?` | |
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/8264?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ransform/function/IdentifierTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSWRlbnRpZmllclRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `47.72% <0.00%> (-24.37%)` | :arrow_down: |
| [...transform/function/IsNotNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOb3ROdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...or/transform/function/IsNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `81.81% <80.00%> (-1.82%)` | :arrow_down: |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `84.52% <100.00%> (-15.48%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/8264/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: |
| [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/8264/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/8264/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/8264/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 [1148 more](https://codecov.io/gh/apache/pinot/pull/8264/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/8264?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/8264?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 [25cd6e3...d7de16f](https://codecov.io/gh/apache/pinot/pull/8264?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] KKcorps commented on a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r832076467
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,96 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.TransformFunctionType;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+
+ private int[] _results;
+ private PeekableIntIterator _nullValueVectorIterator;
+ private NullValueVectorReader _nullValueVectorReader;
+
+ @Override
+ public String getName() {
+ return TransformFunctionType.IS_NOT_NULL.getName();
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1,
+ "Exact 1 argument is required for IS_NOT_NULL operator function");
+ TransformFunction transformFunction = arguments.get(0);
+ if (!(transformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NOT_NULL. Support for functions is planned for future release");
+ }
+ String columnName = ((IdentifierTransformFunction) transformFunction).getColumnName();
+ _nullValueVectorReader = dataSourceMap.get(columnName).getNullValueVector();
+ if (_nullValueVectorReader != null) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ } else {
+ _nullValueVectorIterator = null;
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ if (_results == null || _results.length < length) {
+ _results = new int[length];
+ }
+
+ int[] docIds = projectionBlock.getDocIds();
+
+ Arrays.fill(_results, 1);
+ if (_nullValueVectorIterator != null) {
+ if (!_nullValueVectorIterator.hasNext() || (length > 0 && _nullValueVectorIterator.peekNext() > docIds[0])) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ }
Review comment:
done
--
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 a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r830170091
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNullTransformFunction.java
##########
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+ private PeekableIntIterator _nullValueVectorIterator;
+ private NullValueVectorReader _nullValueVectorReader;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1, "Exact 1 argument is required for IS_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ _nullValueVectorReader = _dataSourceMap.get(columnName).getNullValueVector();
+ if (_nullValueVectorReader != null) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ } else {
+ _nullValueVectorIterator = null;
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ _results = new int[length];
Review comment:
Yes, I was doing it earlier. But won't it cause issues in cases where the value was already set to 1 but is not unset in the current projection block. In case of `IsNotNullTransformFunction` transform function we are doing ` Arrays.fill(_results, 1);` so there we can re-use the old array.
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r816236780
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,69 @@
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NOT_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1,
+ "Exact 1 argument is required for IS_NOT_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NOT_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ if (_results == null || _results.length < length) {
+ _results = new int[length];
+ }
+
+ int[] docIds = projectionBlock.getDocIds();
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ NullValueVectorReader nullValueVector = _dataSourceMap.get(columnName).getNullValueVector();
+
+ if (nullValueVector != null) {
+ for (int idx = 0; idx < length; idx++) {
+ int docId = docIds[idx];
+ if (nullValueVector.isNull(docId)) {
+ _results[idx] = 0;
+ } else {
+ _results[idx] = 1;
+ }
+ }
Review comment:
It would be much more efficient to make the null value vector iterable in a range and set the value to 0 whenever the null value vector has a value.
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#issuecomment-1071747264
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8264?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 [#8264](https://codecov.io/gh/apache/pinot/pull/8264?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7cd61c1) into [master](https://codecov.io/gh/apache/pinot/commit/25cd6e34b7a6b1b013e7181bfe466677cdc0a61d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (25cd6e3) will **decrease** coverage by `5.69%`.
> The diff coverage is `64.70%`.
```diff
@@ Coverage Diff @@
## master #8264 +/- ##
============================================
- Coverage 69.72% 64.03% -5.70%
Complexity 4264 4264
============================================
Files 1639 1596 -43
Lines 85920 84103 -1817
Branches 12922 12733 -189
============================================
- Hits 59906 53853 -6053
- Misses 21842 26360 +4518
+ Partials 4172 3890 -282
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| unittests1 | `66.95% <64.70%> (+<0.01%)` | :arrow_up: |
| unittests2 | `14.18% <0.00%> (-0.02%)` | :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/8264?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `80.90% <60.00%> (-2.73%)` | :arrow_down: |
| [...transform/function/IsNotNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOb3ROdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `63.33% <63.33%> (ø)` | |
| [...or/transform/function/IsNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `63.33% <63.33%> (ø)` | |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...ransform/function/IdentifierTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSWRlbnRpZmllclRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `72.72% <100.00%> (+0.63%)` | :arrow_up: |
| [...va/org/apache/pinot/common/config/NettyConfig.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL05ldHR5Q29uZmlnLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [337 more](https://codecov.io/gh/apache/pinot/pull/8264/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/8264?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/8264?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 [25cd6e3...7cd61c1](https://codecov.io/gh/apache/pinot/pull/8264?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] KKcorps commented on a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r831441999
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
Review comment:
readability is my only concern. I also prefer extracting out the methods. If there was an existing utility class where I could put them I would have. But don't want to create a new class for these 2 only. This check is rarely gonna be used in any other transform function.
--
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 a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r830460885
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NOT_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+ private PeekableIntIterator _nullValueVectorIterator;
+ private NullValueVectorReader _nullValueVectorReader;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1,
+ "Exact 1 argument is required for IS_NOT_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NOT_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ _nullValueVectorReader = _dataSourceMap.get(columnName).getNullValueVector();
+ if (_nullValueVectorReader != null) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ } else {
+ _nullValueVectorIterator = null;
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ if (_results == null || _results.length < length) {
+ _results = new int[length];
+ }
+
+ int[] docIds = projectionBlock.getDocIds();
+
+ if (!_nullValueVectorIterator.hasNext() || (length > 0 && _nullValueVectorIterator.peekNext() > docIds[0])) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ }
Review comment:
The unit tests were failing without it because I pass same projection block here
```java
int[] intValues = transformFunction.transformToIntValuesSV(_projectionBlock);
long[] longValues = transformFunction.transformToLongValuesSV(_projectionBlock);
float[] floatValues = transformFunction.transformToFloatValuesSV(_projectionBlock);
double[] doubleValues = transformFunction.transformToDoubleValuesSV(_projectionBlock);
String[] stringValues = transformFunction.transformToStringValuesSV(_projectionBlock);
```
This scenario can never occur in production 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] richardstartin commented on a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r816681928
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,69 @@
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NOT_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1,
+ "Exact 1 argument is required for IS_NOT_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NOT_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ if (_results == null || _results.length < length) {
+ _results = new int[length];
+ }
+
+ int[] docIds = projectionBlock.getDocIds();
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ NullValueVectorReader nullValueVector = _dataSourceMap.get(columnName).getNullValueVector();
+
+ if (nullValueVector != null) {
+ for (int idx = 0; idx < length; idx++) {
+ int docId = docIds[idx];
+ if (nullValueVector.isNull(docId)) {
+ _results[idx] = 0;
+ } else {
+ _results[idx] = 1;
+ }
+ }
Review comment:
Also, it should be easy to extend `BenchmarkQueries` to test this out
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#issuecomment-1071747264
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8264?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 [#8264](https://codecov.io/gh/apache/pinot/pull/8264?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f0e7618) into [master](https://codecov.io/gh/apache/pinot/commit/25cd6e34b7a6b1b013e7181bfe466677cdc0a61d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (25cd6e3) will **decrease** coverage by `5.72%`.
> The diff coverage is `64.70%`.
```diff
@@ Coverage Diff @@
## master #8264 +/- ##
============================================
- Coverage 69.72% 64.00% -5.73%
- Complexity 4264 4272 +8
============================================
Files 1639 1600 -39
Lines 85920 84243 -1677
Branches 12922 12737 -185
============================================
- Hits 59906 53918 -5988
- Misses 21842 26431 +4589
+ Partials 4172 3894 -278
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| unittests1 | `66.92% <64.70%> (-0.03%)` | :arrow_down: |
| unittests2 | `14.18% <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/8264?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `80.90% <60.00%> (-2.73%)` | :arrow_down: |
| [...transform/function/IsNotNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOb3ROdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `63.33% <63.33%> (ø)` | |
| [...or/transform/function/IsNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `63.33% <63.33%> (ø)` | |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...ransform/function/IdentifierTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSWRlbnRpZmllclRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `72.72% <100.00%> (+0.63%)` | :arrow_up: |
| [...va/org/apache/pinot/common/config/NettyConfig.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL05ldHR5Q29uZmlnLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [353 more](https://codecov.io/gh/apache/pinot/pull/8264/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/8264?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/8264?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 [25cd6e3...f0e7618](https://codecov.io/gh/apache/pinot/pull/8264?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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#issuecomment-1071747264
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8264?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 [#8264](https://codecov.io/gh/apache/pinot/pull/8264?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7cd61c1) into [master](https://codecov.io/gh/apache/pinot/commit/25cd6e34b7a6b1b013e7181bfe466677cdc0a61d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (25cd6e3) will **decrease** coverage by `0.13%`.
> The diff coverage is `69.11%`.
```diff
@@ Coverage Diff @@
## master #8264 +/- ##
============================================
- Coverage 69.72% 69.59% -0.14%
Complexity 4264 4264
============================================
Files 1639 1641 +2
Lines 85920 85984 +64
Branches 12922 12936 +14
============================================
- Hits 59906 59837 -69
- Misses 21842 21961 +119
- Partials 4172 4186 +14
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `27.71% <63.23%> (?)` | |
| unittests1 | `66.95% <64.70%> (+<0.01%)` | :arrow_up: |
| unittests2 | `14.18% <0.00%> (-0.02%)` | :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/8264?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...transform/function/IsNotNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOb3ROdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `66.66% <66.66%> (ø)` | |
| [...or/transform/function/IsNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `66.66% <66.66%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `83.63% <80.00%> (ø)` | |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...ransform/function/IdentifierTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSWRlbnRpZmllclRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `72.72% <100.00%> (+0.63%)` | :arrow_up: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...nverttorawindex/ConvertToRawIndexTaskExecutor.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvY29udmVydHRvcmF3aW5kZXgvQ29udmVydFRvUmF3SW5kZXhUYXNrRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...e/pinot/common/minion/MergeRollupTaskMetadata.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWluaW9uL01lcmdlUm9sbHVwVGFza01ldGFkYXRhLmphdmE=) | `0.00% <0.00%> (-94.74%)` | :arrow_down: |
| [...plugin/segmentuploader/SegmentUploaderDefault.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtcGx1Z2lucy9waW5vdC1zZWdtZW50LXVwbG9hZGVyL3Bpbm90LXNlZ21lbnQtdXBsb2FkZXItZGVmYXVsdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL3NlZ21lbnR1cGxvYWRlci9TZWdtZW50VXBsb2FkZXJEZWZhdWx0LmphdmE=) | `0.00% <0.00%> (-87.10%)` | :arrow_down: |
| [.../transform/function/MapValueTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTWFwVmFsdWVUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `0.00% <0.00%> (-85.30%)` | :arrow_down: |
| ... and [164 more](https://codecov.io/gh/apache/pinot/pull/8264/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/8264?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/8264?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 [25cd6e3...7cd61c1](https://codecov.io/gh/apache/pinot/pull/8264?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] richardstartin commented on a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r816237591
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java
##########
@@ -108,9 +110,9 @@
// Geo indexing
GEOTOH3("geoToH3");
- private static final Set<String> NAMES = Arrays.stream(values())
- .flatMap(func -> Stream.of(func.getName(), StringUtils.remove(func.getName(), '_').toUpperCase(),
- func.getName().toUpperCase(), func.getName().toLowerCase(), func.name(), func.name().toLowerCase()))
+ private static final Set<String> NAMES = Arrays.stream(values()).flatMap(
+ func -> Stream.of(func.getName(), StringUtils.remove(func.getName(), '_').toUpperCase(),
+ func.getName().toUpperCase(), func.getName().toLowerCase(), func.name(), func.name().toLowerCase()))
Review comment:
I don't think this reformatting is necessary or necessarily an improvement.
--
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 a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r816653109
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,69 @@
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NOT_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1,
+ "Exact 1 argument is required for IS_NOT_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NOT_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ if (_results == null || _results.length < length) {
+ _results = new int[length];
+ }
+
+ int[] docIds = projectionBlock.getDocIds();
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ NullValueVectorReader nullValueVector = _dataSourceMap.get(columnName).getNullValueVector();
+
+ if (nullValueVector != null) {
+ for (int idx = 0; idx < length; idx++) {
+ int docId = docIds[idx];
+ if (nullValueVector.isNull(docId)) {
+ _results[idx] = 0;
+ } else {
+ _results[idx] = 1;
+ }
+ }
Review comment:
That will require extra space complexity of O(n) to store docId to index lookup. Or is there any other way?
--
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 a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r830460885
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NOT_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+ private PeekableIntIterator _nullValueVectorIterator;
+ private NullValueVectorReader _nullValueVectorReader;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1,
+ "Exact 1 argument is required for IS_NOT_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NOT_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ _nullValueVectorReader = _dataSourceMap.get(columnName).getNullValueVector();
+ if (_nullValueVectorReader != null) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ } else {
+ _nullValueVectorIterator = null;
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ if (_results == null || _results.length < length) {
+ _results = new int[length];
+ }
+
+ int[] docIds = projectionBlock.getDocIds();
+
+ if (!_nullValueVectorIterator.hasNext() || (length > 0 && _nullValueVectorIterator.peekNext() > docIds[0])) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ }
Review comment:
The unit tests were failing without it because I pass same projection block here
```java
int[] intValues = transformFunction.transformToIntValuesSV(_projectionBlock);
long[] longValues = transformFunction.transformToLongValuesSV(_projectionBlock);
float[] floatValues = transformFunction.transformToFloatValuesSV(_projectionBlock);
double[] doubleValues = transformFunction.transformToDoubleValuesSV(_projectionBlock);
String[] stringValues = transformFunction.transformToStringValuesSV(_projectionBlock);
```
This scenario can never occurs in production 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] codecov-commenter edited a comment on pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#issuecomment-1071747264
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r830054309
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNullTransformFunction.java
##########
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+ private PeekableIntIterator _nullValueVectorIterator;
+ private NullValueVectorReader _nullValueVectorReader;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1, "Exact 1 argument is required for IS_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ _nullValueVectorReader = _dataSourceMap.get(columnName).getNullValueVector();
+ if (_nullValueVectorReader != null) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ } else {
+ _nullValueVectorIterator = null;
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ _results = new int[length];
Review comment:
Shouldn't this check if it can reuse the array? I imagine you did this for a reason.
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
KKcorps commented on pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#issuecomment-1073122898
One more side note - If these functions are used with a table where Null handling is disabled, they assume everything is non-null. So `IS NULL` will always return `false` and `IS NOT NULL` will always return `true`.
Instead, If you want it to throw exception in this case, I can do that as well.
--
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 a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r831470109
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/TransformFunctionFactory.java
##########
@@ -165,6 +165,9 @@ private TransformFunctionFactory() {
// tuple selection
put(canonicalize(TransformFunctionType.LEAST.getName().toLowerCase()), LeastTransformFunction.class);
put(canonicalize(TransformFunctionType.GREATEST.getName().toLowerCase()), GreatestTransformFunction.class);
+ put(canonicalize(TransformFunctionType.IS_NULL.getName().toLowerCase()), IsNullTransformFunction.class);
Review comment:
done
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#issuecomment-1071747264
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8264?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 [#8264](https://codecov.io/gh/apache/pinot/pull/8264?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a62cd28) into [master](https://codecov.io/gh/apache/pinot/commit/e6330bb7297c2d6f80657e4db8619ed34ccfda43?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e6330bb) will **decrease** coverage by `0.03%`.
> The diff coverage is `56.25%`.
```diff
@@ Coverage Diff @@
## master #8264 +/- ##
============================================
- Coverage 64.06% 64.03% -0.04%
- Complexity 4246 4267 +21
============================================
Files 1586 1596 +10
Lines 83399 84107 +708
Branches 12641 12730 +89
============================================
+ Hits 53427 53854 +427
- Misses 26129 26369 +240
- Partials 3843 3884 +41
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests1 | `66.96% <56.25%> (-0.06%)` | :arrow_down: |
| unittests2 | `14.17% <0.00%> (+0.07%)` | :arrow_up: |
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/8264?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...transform/function/IsNotNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOb3ROdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `80.90% <60.00%> (+0.90%)` | :arrow_up: |
| [...or/transform/function/IsNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `65.51% <65.51%> (ø)` | |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...org/apache/pinot/sql/parsers/CalciteSqlParser.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvcGFyc2Vycy9DYWxjaXRlU3FsUGFyc2VyLmphdmE=) | `86.98% <100.00%> (+1.37%)` | :arrow_up: |
| [...a/org/apache/pinot/core/common/DataBlockCache.java](https://codecov.io/gh/apache/pinot/pull/8264/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==) | `88.65% <100.00%> (+0.08%)` | :arrow_up: |
| [...he/pinot/core/operator/blocks/ProjectionBlock.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvUHJvamVjdGlvbkJsb2NrLmphdmE=) | `61.29% <100.00%> (+1.29%)` | :arrow_up: |
| [...che/pinot/core/operator/blocks/TransformBlock.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvVHJhbnNmb3JtQmxvY2suamF2YQ==) | `64.28% <100.00%> (-4.95%)` | :arrow_down: |
| [.../core/operator/query/SelectionOrderByOperator.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9xdWVyeS9TZWxlY3Rpb25PcmRlckJ5T3BlcmF0b3IuamF2YQ==) | `91.44% <100.00%> (-0.50%)` | :arrow_down: |
| [...ransform/function/IdentifierTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSWRlbnRpZmllclRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `72.72% <100.00%> (+0.63%)` | :arrow_up: |
| ... and [114 more](https://codecov.io/gh/apache/pinot/pull/8264/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/8264?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/8264?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 [e6330bb...a62cd28](https://codecov.io/gh/apache/pinot/pull/8264?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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#issuecomment-1071747264
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8264?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 [#8264](https://codecov.io/gh/apache/pinot/pull/8264?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (11fe921) into [master](https://codecov.io/gh/apache/pinot/commit/e6330bb7297c2d6f80657e4db8619ed34ccfda43?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e6330bb) will **decrease** coverage by `0.05%`.
> The diff coverage is `55.34%`.
> :exclamation: Current head 11fe921 differs from pull request most recent head 05d3fcd. Consider uploading reports for the commit 05d3fcd to get more accurate results
```diff
@@ Coverage Diff @@
## master #8264 +/- ##
============================================
- Coverage 64.06% 64.00% -0.06%
- Complexity 4246 4263 +17
============================================
Files 1586 1596 +10
Lines 83399 84111 +712
Branches 12641 12732 +91
============================================
+ Hits 53427 53838 +411
- Misses 26129 26373 +244
- Partials 3843 3900 +57
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests1 | `66.95% <57.32%> (-0.07%)` | :arrow_down: |
| unittests2 | `14.15% <19.77%> (+0.06%)` | :arrow_up: |
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/8264?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...roker/requesthandler/GrpcBrokerRequestHandler.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvR3JwY0Jyb2tlclJlcXVlc3RIYW5kbGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...va/org/apache/pinot/common/config/NettyConfig.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL05ldHR5Q29uZmlnLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...pache/pinot/common/config/provider/TableCache.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL3Byb3ZpZGVyL1RhYmxlQ2FjaGUuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...apache/pinot/common/helix/ExtraInstanceConfig.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vaGVsaXgvRXh0cmFJbnN0YW5jZUNvbmZpZy5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...g/apache/pinot/common/metrics/ControllerGauge.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyR2F1Z2UuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...g/apache/pinot/common/utils/helix/HelixHelper.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvaGVsaXgvSGVsaXhIZWxwZXIuamF2YQ==) | `13.30% <0.00%> (-2.72%)` | :arrow_down: |
| [.../org/apache/pinot/core/common/MinionConstants.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vTWluaW9uQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (ø)` | |
| [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `50.00% <ø> (ø)` | |
| [...ata/manager/realtime/RealtimeTableDataManager.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVUYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `11.64% <ø> (ø)` | |
| [.../pinot/core/operator/filter/NotFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvTm90RmlsdGVyT3BlcmF0b3IuamF2YQ==) | `66.66% <ø> (+11.11%)` | :arrow_up: |
| ... and [178 more](https://codecov.io/gh/apache/pinot/pull/8264/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/8264?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/8264?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 [e6330bb...05d3fcd](https://codecov.io/gh/apache/pinot/pull/8264?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 commented on pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#issuecomment-1071747264
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8264?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 [#8264](https://codecov.io/gh/apache/pinot/pull/8264?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a62cd28) into [master](https://codecov.io/gh/apache/pinot/commit/e6330bb7297c2d6f80657e4db8619ed34ccfda43?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e6330bb) will **decrease** coverage by `49.89%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #8264 +/- ##
=============================================
- Coverage 64.06% 14.17% -49.90%
+ Complexity 4246 81 -4165
=============================================
Files 1586 1596 +10
Lines 83399 84107 +708
Branches 12641 12730 +89
=============================================
- Hits 53427 11919 -41508
- Misses 26129 71292 +45163
+ Partials 3843 896 -2947
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests1 | `?` | |
| unittests2 | `14.17% <0.00%> (+0.07%)` | :arrow_up: |
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/8264?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...org/apache/pinot/sql/parsers/CalciteSqlParser.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvcGFyc2Vycy9DYWxjaXRlU3FsUGFyc2VyLmphdmE=) | `0.00% <0.00%> (-85.62%)` | :arrow_down: |
| [...a/org/apache/pinot/core/common/DataBlockCache.java](https://codecov.io/gh/apache/pinot/pull/8264/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%> (-88.58%)` | :arrow_down: |
| [...he/pinot/core/operator/blocks/ProjectionBlock.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvUHJvamVjdGlvbkJsb2NrLmphdmE=) | `0.00% <0.00%> (-60.00%)` | :arrow_down: |
| [...che/pinot/core/operator/blocks/TransformBlock.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvVHJhbnNmb3JtQmxvY2suamF2YQ==) | `0.00% <0.00%> (-69.24%)` | :arrow_down: |
| [.../core/operator/query/SelectionOrderByOperator.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9xdWVyeS9TZWxlY3Rpb25PcmRlckJ5T3BlcmF0b3IuamF2YQ==) | `0.00% <0.00%> (-91.94%)` | :arrow_down: |
| [...ransform/function/IdentifierTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSWRlbnRpZmllclRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `0.00% <0.00%> (-72.10%)` | :arrow_down: |
| [...transform/function/IsNotNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOb3ROdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...or/transform/function/IsNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `0.00% <0.00%> (-80.00%)` | :arrow_down: |
| ... and [1139 more](https://codecov.io/gh/apache/pinot/pull/8264/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/8264?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/8264?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 [e6330bb...a62cd28](https://codecov.io/gh/apache/pinot/pull/8264?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] richardstartin commented on a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r816656089
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,69 @@
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NOT_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1,
+ "Exact 1 argument is required for IS_NOT_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NOT_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ if (_results == null || _results.length < length) {
+ _results = new int[length];
+ }
+
+ int[] docIds = projectionBlock.getDocIds();
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ NullValueVectorReader nullValueVector = _dataSourceMap.get(columnName).getNullValueVector();
+
+ if (nullValueVector != null) {
+ for (int idx = 0; idx < length; idx++) {
+ int docId = docIds[idx];
+ if (nullValueVector.isNull(docId)) {
+ _results[idx] = 0;
+ } else {
+ _results[idx] = 1;
+ }
+ }
Review comment:
No it won't - the null value vector is just a bitmap. If docId x is null, bit x is set in the null value vector.
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#issuecomment-1071747264
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8264?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 [#8264](https://codecov.io/gh/apache/pinot/pull/8264?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7cd61c1) into [master](https://codecov.io/gh/apache/pinot/commit/25cd6e34b7a6b1b013e7181bfe466677cdc0a61d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (25cd6e3) will **increase** coverage by `1.06%`.
> The diff coverage is `69.11%`.
```diff
@@ Coverage Diff @@
## master #8264 +/- ##
============================================
+ Coverage 69.72% 70.78% +1.06%
Complexity 4264 4264
============================================
Files 1639 1641 +2
Lines 85920 85984 +64
Branches 12922 12936 +14
============================================
+ Hits 59906 60863 +957
+ Misses 21842 20910 -932
- Partials 4172 4211 +39
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `28.86% <8.82%> (-0.01%)` | :arrow_down: |
| integration2 | `27.71% <63.23%> (?)` | |
| unittests1 | `66.95% <64.70%> (+<0.01%)` | :arrow_up: |
| unittests2 | `14.18% <0.00%> (-0.02%)` | :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/8264?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...transform/function/IsNotNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOb3ROdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `66.66% <66.66%> (ø)` | |
| [...or/transform/function/IsNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `66.66% <66.66%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `83.63% <80.00%> (ø)` | |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...ransform/function/IdentifierTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSWRlbnRpZmllclRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `72.72% <100.00%> (+0.63%)` | :arrow_up: |
| [...mmon/request/context/predicate/NotEqPredicate.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L3ByZWRpY2F0ZS9Ob3RFcVByZWRpY2F0ZS5qYXZh) | `66.66% <0.00%> (-6.67%)` | :arrow_down: |
| [...nction/DistinctCountBitmapAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9EaXN0aW5jdENvdW50Qml0bWFwQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `54.40% <0.00%> (-1.04%)` | :arrow_down: |
| [...ot/core/query/pruner/ColumnValueSegmentPruner.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9wcnVuZXIvQ29sdW1uVmFsdWVTZWdtZW50UHJ1bmVyLmphdmE=) | `74.33% <0.00%> (-0.54%)` | :arrow_down: |
| [.../aggregation/function/ModeAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9Nb2RlQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `88.37% <0.00%> (-0.28%)` | :arrow_down: |
| [.../pinot/controller/recommender/io/InputManager.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9pby9JbnB1dE1hbmFnZXIuamF2YQ==) | `91.63% <0.00%> (+0.03%)` | :arrow_up: |
| ... and [87 more](https://codecov.io/gh/apache/pinot/pull/8264/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/8264?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/8264?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 [25cd6e3...7cd61c1](https://codecov.io/gh/apache/pinot/pull/8264?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] KKcorps commented on a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r832076309
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NOT_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+ private PeekableIntIterator _nullValueVectorIterator;
+ private NullValueVectorReader _nullValueVectorReader;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1,
+ "Exact 1 argument is required for IS_NOT_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NOT_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ _nullValueVectorReader = _dataSourceMap.get(columnName).getNullValueVector();
+ if (_nullValueVectorReader != null) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ } else {
+ _nullValueVectorIterator = null;
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ if (_results == null || _results.length < length) {
+ _results = new int[length];
+ }
+
+ int[] docIds = projectionBlock.getDocIds();
+
+ if (!_nullValueVectorIterator.hasNext() || (length > 0 && _nullValueVectorIterator.peekNext() > docIds[0])) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ }
Review comment:
done
--
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 a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r830508504
##########
File path: pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
##########
@@ -610,6 +610,10 @@ private static Expression toExpression(SqlNode node) {
return RequestUtils.getIdentifierExpression(node.toString());
case LITERAL:
return RequestUtils.getLiteralExpression((SqlLiteral) node);
+ case IS_NULL:
Review comment:
done
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r816236985
##########
File path: pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
##########
@@ -613,6 +613,20 @@ private static Expression toExpression(SqlNode node) {
return RequestUtils.getIdentifierExpression(node.toString());
case LITERAL:
return RequestUtils.getLiteralExpression((SqlLiteral) node);
+ case IS_NULL:
+ SqlBasicCall isNullSQLNode = (SqlBasicCall) node;
+ List<SqlNode> isNullOperands = isNullSQLNode.getOperandList();
+ Expression isNullLeftExpr = toExpression(isNullOperands.get(0));
+ final Expression isNullExpr = RequestUtils.getFunctionExpression("IS_NULL");
+ isNullExpr.getFunctionCall().addToOperands(isNullLeftExpr);
+ return isNullExpr;
+ case IS_NOT_NULL:
+ SqlBasicCall isNotNullSQLNode = (SqlBasicCall) node;
+ List<SqlNode> isNotNullOperands = isNotNullSQLNode.getOperandList();
+ Expression isNotNullLeftExpr = toExpression(isNotNullOperands.get(0));
+ final Expression isNotNullExpr = RequestUtils.getFunctionExpression("IS_NOT_NULL");
+ isNotNullExpr.getFunctionCall().addToOperands(isNotNullLeftExpr);
+ return isNotNullExpr;
Review comment:
Can you extract out a method please?
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r816680897
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,69 @@
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NOT_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1,
+ "Exact 1 argument is required for IS_NOT_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NOT_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ if (_results == null || _results.length < length) {
+ _results = new int[length];
+ }
+
+ int[] docIds = projectionBlock.getDocIds();
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ NullValueVectorReader nullValueVector = _dataSourceMap.get(columnName).getNullValueVector();
+
+ if (nullValueVector != null) {
+ for (int idx = 0; idx < length; idx++) {
+ int docId = docIds[idx];
+ if (nullValueVector.isNull(docId)) {
+ _results[idx] = 0;
+ } else {
+ _results[idx] = 1;
+ }
+ }
Review comment:
Try something like this:
```java
if (nullValueVector != null) {
PeekableIntIterator it = nullValueVector.getNullBitmap().getIntIterator();
int pos = 0;
while (it.hasNext() & pos < length) {
it.advanceIfNeeded(docIds[pos]);
pos = Arrays.binarySearch(docIds, pos, length, it.next());
if (pos >= 0) {
_results[pos] = 0;
pos++;
} else {
pos = -pos - 1;
}
}
}
```
Since blocks are iterated in ascending docId order, it would be better to store the iterator in the TransformFunction which would avoid mapping and advancing the bitmap once per block (currently this code maps the bitmap once per row).
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#issuecomment-1071747264
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8264?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 [#8264](https://codecov.io/gh/apache/pinot/pull/8264?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a62cd28) into [master](https://codecov.io/gh/apache/pinot/commit/e6330bb7297c2d6f80657e4db8619ed34ccfda43?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e6330bb) will **decrease** coverage by `49.89%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #8264 +/- ##
=============================================
- Coverage 64.06% 14.17% -49.90%
+ Complexity 4246 81 -4165
=============================================
Files 1586 1596 +10
Lines 83399 84107 +708
Branches 12641 12730 +89
=============================================
- Hits 53427 11919 -41508
- Misses 26129 71292 +45163
+ Partials 3843 896 -2947
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests1 | `?` | |
| unittests2 | `14.17% <0.00%> (+0.07%)` | :arrow_up: |
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/8264?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...org/apache/pinot/sql/parsers/CalciteSqlParser.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvcGFyc2Vycy9DYWxjaXRlU3FsUGFyc2VyLmphdmE=) | `0.00% <0.00%> (-85.62%)` | :arrow_down: |
| [...a/org/apache/pinot/core/common/DataBlockCache.java](https://codecov.io/gh/apache/pinot/pull/8264/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%> (-88.58%)` | :arrow_down: |
| [...he/pinot/core/operator/blocks/ProjectionBlock.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvUHJvamVjdGlvbkJsb2NrLmphdmE=) | `0.00% <0.00%> (-60.00%)` | :arrow_down: |
| [...che/pinot/core/operator/blocks/TransformBlock.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvVHJhbnNmb3JtQmxvY2suamF2YQ==) | `0.00% <0.00%> (-69.24%)` | :arrow_down: |
| [.../core/operator/query/SelectionOrderByOperator.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9xdWVyeS9TZWxlY3Rpb25PcmRlckJ5T3BlcmF0b3IuamF2YQ==) | `0.00% <0.00%> (-91.94%)` | :arrow_down: |
| [...ransform/function/IdentifierTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSWRlbnRpZmllclRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `0.00% <0.00%> (-72.10%)` | :arrow_down: |
| [...transform/function/IsNotNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOb3ROdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...or/transform/function/IsNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `0.00% <0.00%> (-80.00%)` | :arrow_down: |
| ... and [1139 more](https://codecov.io/gh/apache/pinot/pull/8264/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/8264?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/8264?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 [e6330bb...a62cd28](https://codecov.io/gh/apache/pinot/pull/8264?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] Jackie-Jiang merged pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #8264:
URL: https://github.com/apache/pinot/pull/8264
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r831523383
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,96 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.TransformFunctionType;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+
+ private int[] _results;
+ private PeekableIntIterator _nullValueVectorIterator;
+ private NullValueVectorReader _nullValueVectorReader;
+
+ @Override
+ public String getName() {
+ return TransformFunctionType.IS_NOT_NULL.getName();
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1,
+ "Exact 1 argument is required for IS_NOT_NULL operator function");
+ TransformFunction transformFunction = arguments.get(0);
+ if (!(transformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NOT_NULL. Support for functions is planned for future release");
+ }
+ String columnName = ((IdentifierTransformFunction) transformFunction).getColumnName();
+ _nullValueVectorReader = dataSourceMap.get(columnName).getNullValueVector();
+ if (_nullValueVectorReader != null) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ } else {
+ _nullValueVectorIterator = null;
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ if (_results == null || _results.length < length) {
+ _results = new int[length];
+ }
+
+ int[] docIds = projectionBlock.getDocIds();
+
+ Arrays.fill(_results, 1);
+ if (_nullValueVectorIterator != null) {
+ if (!_nullValueVectorIterator.hasNext() || (length > 0 && _nullValueVectorIterator.peekNext() > docIds[0])) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ }
Review comment:
This can be removed which should improve the performance especially when there is no remaining null values. See the reply in another thread
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#issuecomment-1071747264
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8264?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 [#8264](https://codecov.io/gh/apache/pinot/pull/8264?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d7de16f) into [master](https://codecov.io/gh/apache/pinot/commit/25cd6e34b7a6b1b013e7181bfe466677cdc0a61d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (25cd6e3) will **decrease** coverage by `39.01%`.
> The diff coverage is `67.64%`.
```diff
@@ Coverage Diff @@
## master #8264 +/- ##
=============================================
- Coverage 69.72% 30.70% -39.02%
=============================================
Files 1639 1629 -10
Lines 85920 85630 -290
Branches 12922 12898 -24
=============================================
- Hits 59906 26292 -33614
- Misses 21842 56958 +35116
+ Partials 4172 2380 -1792
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `28.90% <8.82%> (+0.03%)` | :arrow_up: |
| integration2 | `27.57% <67.64%> (?)` | |
| unittests1 | `?` | |
| unittests2 | `?` | |
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/8264?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...or/transform/function/IsNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `63.33% <63.33%> (ø)` | |
| [...transform/function/IsNotNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOb3ROdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `66.66% <66.66%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `81.81% <80.00%> (-1.82%)` | :arrow_down: |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `84.52% <100.00%> (-15.48%)` | :arrow_down: |
| [...ransform/function/IdentifierTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSWRlbnRpZmllclRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `54.54% <100.00%> (-17.55%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/8264/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: |
| [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/8264/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/8264/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/8264/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 [1177 more](https://codecov.io/gh/apache/pinot/pull/8264/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/8264?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/8264?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 [25cd6e3...d7de16f](https://codecov.io/gh/apache/pinot/pull/8264?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] KKcorps commented on a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r830473429
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
Review comment:
I thought about that earlier on but creating a base class / utility class for the functions which most likely are going to be used by only these 2 classes doesn't seem right.
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r830172170
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNullTransformFunction.java
##########
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+ private PeekableIntIterator _nullValueVectorIterator;
+ private NullValueVectorReader _nullValueVectorReader;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1, "Exact 1 argument is required for IS_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ _nullValueVectorReader = _dataSourceMap.get(columnName).getNullValueVector();
+ if (_nullValueVectorReader != null) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ } else {
+ _nullValueVectorIterator = null;
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ _results = new int[length];
Review comment:
just fill the array with zero then, it's better than allocating once per block.
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#issuecomment-1071747264
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8264?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 [#8264](https://codecov.io/gh/apache/pinot/pull/8264?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8449ece) into [master](https://codecov.io/gh/apache/pinot/commit/e6330bb7297c2d6f80657e4db8619ed34ccfda43?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e6330bb) will **increase** coverage by `2.91%`.
> The diff coverage is `57.40%`.
> :exclamation: Current head 8449ece differs from pull request most recent head 983a336. Consider uploading reports for the commit 983a336 to get more accurate results
```diff
@@ Coverage Diff @@
## master #8264 +/- ##
============================================
+ Coverage 64.06% 66.97% +2.91%
+ Complexity 4246 4184 -62
============================================
Files 1586 1239 -347
Lines 83399 62623 -20776
Branches 12641 9740 -2901
============================================
- Hits 53427 41942 -11485
+ Misses 26129 17685 -8444
+ Partials 3843 2996 -847
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests1 | `66.97% <57.40%> (-0.05%)` | :arrow_down: |
| unittests2 | `?` | |
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/8264?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...va/org/apache/pinot/common/config/NettyConfig.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL05ldHR5Q29uZmlnLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...pache/pinot/common/config/provider/TableCache.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL3Byb3ZpZGVyL1RhYmxlQ2FjaGUuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...apache/pinot/common/helix/ExtraInstanceConfig.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vaGVsaXgvRXh0cmFJbnN0YW5jZUNvbmZpZy5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...g/apache/pinot/common/metrics/ControllerGauge.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyR2F1Z2UuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...g/apache/pinot/common/utils/helix/HelixHelper.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvaGVsaXgvSGVsaXhIZWxwZXIuamF2YQ==) | `13.30% <0.00%> (-2.72%)` | :arrow_down: |
| [.../org/apache/pinot/core/common/MinionConstants.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vTWluaW9uQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (ø)` | |
| [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `50.00% <ø> (ø)` | |
| [...ata/manager/realtime/RealtimeTableDataManager.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVUYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `11.64% <ø> (ø)` | |
| [.../pinot/core/operator/filter/NotFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvTm90RmlsdGVyT3BlcmF0b3IuamF2YQ==) | `66.66% <ø> (+11.11%)` | :arrow_up: |
| [...ry/optimizer/statement/JsonStatementOptimizer.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvc3RhdGVtZW50L0pzb25TdGF0ZW1lbnRPcHRpbWl6ZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| ... and [496 more](https://codecov.io/gh/apache/pinot/pull/8264/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/8264?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/8264?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 [e6330bb...983a336](https://codecov.io/gh/apache/pinot/pull/8264?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] richardstartin commented on a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r830054309
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNullTransformFunction.java
##########
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+ private PeekableIntIterator _nullValueVectorIterator;
+ private NullValueVectorReader _nullValueVectorReader;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1, "Exact 1 argument is required for IS_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ _nullValueVectorReader = _dataSourceMap.get(columnName).getNullValueVector();
+ if (_nullValueVectorReader != null) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ } else {
+ _nullValueVectorIterator = null;
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ _results = new int[length];
Review comment:
Shouldn't this check if it can reuse the array? I imagine you did this for a reason.
--
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 a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r830460885
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NOT_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+ private PeekableIntIterator _nullValueVectorIterator;
+ private NullValueVectorReader _nullValueVectorReader;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1,
+ "Exact 1 argument is required for IS_NOT_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NOT_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ _nullValueVectorReader = _dataSourceMap.get(columnName).getNullValueVector();
+ if (_nullValueVectorReader != null) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ } else {
+ _nullValueVectorIterator = null;
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ if (_results == null || _results.length < length) {
+ _results = new int[length];
+ }
+
+ int[] docIds = projectionBlock.getDocIds();
+
+ if (!_nullValueVectorIterator.hasNext() || (length > 0 && _nullValueVectorIterator.peekNext() > docIds[0])) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ }
Review comment:
The unit tests were failing without it because I pass same projection block here
```java
int[] intValues = transformFunction.transformToIntValuesSV(_projectionBlock);
long[] longValues = transformFunction.transformToLongValuesSV(_projectionBlock);
float[] floatValues = transformFunction.transformToFloatValuesSV(_projectionBlock);
double[] doubleValues = transformFunction.transformToDoubleValuesSV(_projectionBlock);
String[] stringValues = transformFunction.transformToStringValuesSV(_projectionBlock);
```
Should I recreate transform function for each of these? This scenario can never occur in production 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] KKcorps commented on a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r830460885
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NOT_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+ private PeekableIntIterator _nullValueVectorIterator;
+ private NullValueVectorReader _nullValueVectorReader;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1,
+ "Exact 1 argument is required for IS_NOT_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NOT_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ _nullValueVectorReader = _dataSourceMap.get(columnName).getNullValueVector();
+ if (_nullValueVectorReader != null) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ } else {
+ _nullValueVectorIterator = null;
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ if (_results == null || _results.length < length) {
+ _results = new int[length];
+ }
+
+ int[] docIds = projectionBlock.getDocIds();
+
+ if (!_nullValueVectorIterator.hasNext() || (length > 0 && _nullValueVectorIterator.peekNext() > docIds[0])) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ }
Review comment:
The unit tests were failing without it because I pass same projection block here
```java
int[] intValues = transformFunction.transformToIntValuesSV(_projectionBlock);
long[] longValues = transformFunction.transformToLongValuesSV(_projectionBlock);
float[] floatValues = transformFunction.transformToFloatValuesSV(_projectionBlock);
double[] doubleValues = transformFunction.transformToDoubleValuesSV(_projectionBlock);
String[] stringValues = transformFunction.transformToStringValuesSV(_projectionBlock);
```
This scenario can never occur in production 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] KKcorps commented on a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r830508467
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NOT_NULL";
Review comment:
done
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NOT_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+ private PeekableIntIterator _nullValueVectorIterator;
+ private NullValueVectorReader _nullValueVectorReader;
Review comment:
done
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#issuecomment-1071747264
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8264?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 [#8264](https://codecov.io/gh/apache/pinot/pull/8264?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f0e7618) into [master](https://codecov.io/gh/apache/pinot/commit/25cd6e34b7a6b1b013e7181bfe466677cdc0a61d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (25cd6e3) will **decrease** coverage by `55.53%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #8264 +/- ##
=============================================
- Coverage 69.72% 14.18% -55.54%
+ Complexity 4264 81 -4183
=============================================
Files 1639 1600 -39
Lines 85920 84243 -1677
Branches 12922 12737 -185
=============================================
- Hits 59906 11950 -47956
- Misses 21842 71394 +49552
+ Partials 4172 899 -3273
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.18% <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/8264?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ransform/function/IdentifierTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSWRlbnRpZmllclRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `0.00% <0.00%> (-72.10%)` | :arrow_down: |
| [...transform/function/IsNotNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOb3ROdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...or/transform/function/IsNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `0.00% <0.00%> (-83.64%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/8264/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/8264/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/8264/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/8264/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/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/8264/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1293 more](https://codecov.io/gh/apache/pinot/pull/8264/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/8264?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/8264?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 [25cd6e3...f0e7618](https://codecov.io/gh/apache/pinot/pull/8264?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] Jackie-Jiang commented on a change in pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r831430275
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
+ public static final String FUNCTION_NAME = "IS_NOT_NULL";
+
+ private TransformFunction _leftTransformFunction;
+ private int[] _results;
+ private Map<String, DataSource> _dataSourceMap = new HashMap<>();
+ private PeekableIntIterator _nullValueVectorIterator;
+ private NullValueVectorReader _nullValueVectorReader;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ Preconditions.checkArgument(arguments.size() == 1,
+ "Exact 1 argument is required for IS_NOT_NULL operator function");
+ _leftTransformFunction = arguments.get(0);
+ if (!(_leftTransformFunction instanceof IdentifierTransformFunction)) {
+ throw new IllegalArgumentException(
+ "Only column names are supported in IS_NOT_NULL. Support for functions is planned for future release");
+ }
+ _dataSourceMap = dataSourceMap;
+ String columnName = ((IdentifierTransformFunction) _leftTransformFunction).getColumnName();
+ _nullValueVectorReader = _dataSourceMap.get(columnName).getNullValueVector();
+ if (_nullValueVectorReader != null) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ } else {
+ _nullValueVectorIterator = null;
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BOOLEAN_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ if (_results == null || _results.length < length) {
+ _results = new int[length];
+ }
+
+ int[] docIds = projectionBlock.getDocIds();
+
+ if (!_nullValueVectorIterator.hasNext() || (length > 0 && _nullValueVectorIterator.peekNext() > docIds[0])) {
+ _nullValueVectorIterator = _nullValueVectorReader.getNullBitmap().getIntIterator();
+ }
Review comment:
Yes, each `ProjectionBlock` should be processed only per transform function. We should simplify the handling to avoid overhead, and fix the test instead
--
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 #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#discussion_r831432053
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.PeekableIntIterator;
+
+
+public class IsNotNullTransformFunction extends BaseTransformFunction {
Review comment:
Is the concern more about performance or readability? I prefer extracting the common methods when it is not very small (less than 5 lines) so that future changes do not need to go to multiple places. But I'm okay either way because as you said there are only 2 classes
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/TransformFunctionFactory.java
##########
@@ -165,6 +165,9 @@ private TransformFunctionFactory() {
// tuple selection
put(canonicalize(TransformFunctionType.LEAST.getName().toLowerCase()), LeastTransformFunction.class);
put(canonicalize(TransformFunctionType.GREATEST.getName().toLowerCase()), GreatestTransformFunction.class);
+ put(canonicalize(TransformFunctionType.IS_NULL.getName().toLowerCase()), IsNullTransformFunction.class);
Review comment:
(minor) Add a new category for them as they are not tuple selection
--
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 pull request #8264: Add support for IS NULL and NOT IS NULL in transform functions
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on pull request #8264:
URL: https://github.com/apache/pinot/pull/8264#issuecomment-1074274131
> One more side note - If these functions are used with a table where Null handling is disabled, they assume everything is non-null. So `IS NULL` will always return `false` and `IS NOT NULL` will always return `true`.
This is the correct behavior which aligns with the filter semantic
--
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