[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #4993: Support Text Search (both offline and realtime)
siddharthteotia commented on a change in pull request #4993: Support Text
Search (both offline and realtime)
URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r367795327
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/TextMatchFilterOperator.java
##
@@ -0,0 +1,67 @@
+/**
+ * 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.filter;
+
+import com.google.common.base.Preconditions;
+import org.apache.pinot.core.common.DataSource;
+import org.apache.pinot.core.common.Predicate;
+import org.apache.pinot.core.operator.blocks.FilterBlock;
+import org.apache.pinot.core.operator.docidsets.LuceneIndexDocIdSet;
+import org.apache.pinot.core.operator.filter.predicate.PredicateEvaluator;
+import
org.apache.pinot.core.operator.filter.predicate.TextMatchPredicateEvaluatorFactory;
+import org.apache.pinot.core.segment.index.readers.InvertedIndexReader;
+import org.apache.pinot.core.segment.index.readers.text.LuceneTextIndexReader;
+
+
+/**
+ * Filter operator for supporting the execution of text search
+ * queries: WHERE TEXT_MATCH(column_name, query_string)
+ */
+public class TextMatchFilterOperator extends BaseFilterOperator {
+ private static final String OPERATOR_NAME = "TextMatchFilterOperator";
+
+ private final Predicate _predicate;
+ private final DataSource _dataSource;
+ private final int _startDocId;
+ private final int _endDocId;
+
+ public TextMatchFilterOperator(PredicateEvaluator predicateEvaluator,
DataSource dataSource, int startDocId, int endDocId) {
+Preconditions.checkArgument(predicateEvaluator instanceof
TextMatchPredicateEvaluatorFactory.RawValueBasedTextMatchPredicateEvaluator &&
+!predicateEvaluator.isAlwaysTrue() && !predicateEvaluator.isAlwaysFalse());
+
TextMatchPredicateEvaluatorFactory.RawValueBasedTextMatchPredicateEvaluator
evaluator =
(TextMatchPredicateEvaluatorFactory.RawValueBasedTextMatchPredicateEvaluator)predicateEvaluator;
+_predicate = evaluator.getPredicate();
+_dataSource = dataSource;
+_startDocId = startDocId;
+_endDocId = endDocId;
+ }
+
+ @Override
+ protected FilterBlock getNextBlock() {
+InvertedIndexReader textIndexReader = _dataSource.getInvertedIndex();
+Preconditions.checkNotNull(textIndexReader, "Error: expecting non-null
text index");
+LuceneTextIndexReader.LuceneSearchResult luceneSearchResult =
(LuceneTextIndexReader.LuceneSearchResult)textIndexReader.getDocIds(_predicate);
+// TODO: For the next iteration of this feature, consider using
BitMapDocIdSet (and BitMapDocIdIterator)
Review comment:
I have pushed a new commit doing this. However, there is a performance
concern associated with it. Let's discuss that. Details mentioned in this
comment
https://github.com/apache/incubator-pinot/pull/4993#discussion_r367800344
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #4993: Support Text Search (both offline and realtime)
siddharthteotia commented on a change in pull request #4993: Support Text
Search (both offline and realtime)
URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r367830796
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/TextMatchFilterOperator.java
##
@@ -0,0 +1,67 @@
+/**
+ * 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.filter;
+
+import com.google.common.base.Preconditions;
+import org.apache.pinot.core.common.DataSource;
+import org.apache.pinot.core.common.Predicate;
+import org.apache.pinot.core.operator.blocks.FilterBlock;
+import org.apache.pinot.core.operator.docidsets.LuceneIndexDocIdSet;
+import org.apache.pinot.core.operator.filter.predicate.PredicateEvaluator;
+import
org.apache.pinot.core.operator.filter.predicate.TextMatchPredicateEvaluatorFactory;
+import org.apache.pinot.core.segment.index.readers.InvertedIndexReader;
+import org.apache.pinot.core.segment.index.readers.text.LuceneTextIndexReader;
+
+
+/**
+ * Filter operator for supporting the execution of text search
+ * queries: WHERE TEXT_MATCH(column_name, query_string)
+ */
+public class TextMatchFilterOperator extends BaseFilterOperator {
+ private static final String OPERATOR_NAME = "TextMatchFilterOperator";
+
+ private final Predicate _predicate;
+ private final DataSource _dataSource;
+ private final int _startDocId;
+ private final int _endDocId;
+
+ public TextMatchFilterOperator(PredicateEvaluator predicateEvaluator,
DataSource dataSource, int startDocId, int endDocId) {
+Preconditions.checkArgument(predicateEvaluator instanceof
TextMatchPredicateEvaluatorFactory.RawValueBasedTextMatchPredicateEvaluator &&
+!predicateEvaluator.isAlwaysTrue() && !predicateEvaluator.isAlwaysFalse());
+
TextMatchPredicateEvaluatorFactory.RawValueBasedTextMatchPredicateEvaluator
evaluator =
(TextMatchPredicateEvaluatorFactory.RawValueBasedTextMatchPredicateEvaluator)predicateEvaluator;
+_predicate = evaluator.getPredicate();
+_dataSource = dataSource;
+_startDocId = startDocId;
+_endDocId = endDocId;
+ }
+
+ @Override
+ protected FilterBlock getNextBlock() {
+InvertedIndexReader textIndexReader = _dataSource.getInvertedIndex();
+Preconditions.checkNotNull(textIndexReader, "Error: expecting non-null
text index");
+LuceneTextIndexReader.LuceneSearchResult luceneSearchResult =
(LuceneTextIndexReader.LuceneSearchResult)textIndexReader.getDocIds(_predicate);
+// TODO: For the next iteration of this feature, consider using
BitMapDocIdSet (and BitMapDocIdIterator)
Review comment:
Discussed offline. Using BitMapDocIdSet is better for general cases.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4519: Add support sql query http api
codecov-io edited a comment on issue #4519: Add support sql query http api URL: https://github.com/apache/incubator-pinot/pull/4519#issuecomment-520156371 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4519?src=pr&el=h1) Report > :exclamation: No coverage uploaded for pull request base (`master@aea8fca`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit). > The diff coverage is `0%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4519?src=pr&el=tree) ```diff @@Coverage Diff@@ ## master#4519 +/- ## = Coverage ? 56.41% Complexity? 20 = Files ? 1075 Lines ?55968 Branches ? 8173 = Hits ?31576 Misses?21886 Partials ? 2506 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4519?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...org/apache/pinot/common/utils/CommonConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/4519/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `34.61% <ø> (ø)` | `0 <0> (?)` | | | [...t/controller/api/resources/PinotQueryResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/4519/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90UXVlcnlSZXNvdXJjZS5qYXZh) | `0% <0%> (ø)` | `0 <0> (?)` | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4519?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4519?src=pr&el=footer). Last update [aea8fca...cb6f059](https://codecov.io/gh/apache/incubator-pinot/pull/4519?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 merged pull request #4987: Pinot batch ingestion hadoop
fx19880617 merged pull request #4987: Pinot batch ingestion hadoop URL: https://github.com/apache/incubator-pinot/pull/4987 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated (7ff9dd4 -> f990656)
This is an automated email from the ASF dual-hosted git repository.
xiangfu pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git.
from 7ff9dd4 [TE] Fix over-scheduling tasks in data availability scheduler
(#4984)
add f990656 Pinot batch ingestion hadoop (#4987)
No new revisions were added by this update.
Summary of changes:
docs/batch_data_ingestion.rst | 18 ++
pinot-distribution/pinot-assembly.xml | 2 +
pinot-distribution/pom.xml | 7 +
.../pinot-batch-ingestion-common/pom.xml | 13 -
.../batch/common/SegmentGenerationUtils.java | 150 ++
.../ingestion/batch/common/SegmentPushUtils.java | 8 +-
.../pinot-batch-ingestion-hadoop}/pom.xml | 125
.../batch/hadoop/HadoopSegmentCreationMapper.java | 227 +++
.../hadoop/HadoopSegmentGenerationJobRunner.java | 323 +
.../hadoop/HadoopSegmentTarPushJobRunner.java} | 17 +-
.../hadoop/HadoopSegmentUriPushJobRunner.java} | 12 +-
.../segmentCreationAndTarPushJobSpec.yaml | 8 +-
.../segmentCreationAndUriPushJobSpec.yaml | 8 +-
.../src/main/resources/segmentCreationJobSpec.yaml | 8 +-
.../src/main/resources/segmentTarPushJobSpec.yaml | 8 +-
.../src/main/resources/segmentUriPushJobSpec.yaml | 8 +-
.../pinot-batch-ingestion-spark/pom.xml| 17 --
.../spark/SparkSegmentGenerationJobRunner.java | 194 +
.../pinot-batch-ingestion-standalone/pom.xml | 17 --
.../standalone/SegmentGenerationJobRunner.java | 134 +
pinot-plugins/pinot-batch-ingestion/pom.xml| 2 +-
.../v0_deprecated/pinot-hadoop/pom.xml | 4 -
.../v0_deprecated/pinot-ingestion-common/pom.xml | 17 --
.../v0_deprecated/pinot-spark/pom.xml | 17 --
pinot-plugins/pinot-file-system/pinot-hdfs/pom.xml | 8 -
.../pinot-input-format/pinot-avro/pom.xml | 8 -
pinot-plugins/pinot-input-format/pinot-csv/pom.xml | 8 -
.../pinot-input-format/pinot-json/pom.xml | 8 -
pinot-plugins/pinot-input-format/pinot-orc/pom.xml | 8 -
.../pinot-input-format/pinot-parquet/pom.xml | 8 -
.../pinot-input-format/pinot-thrift/pom.xml| 8 -
pinot-plugins/pom.xml | 11 +
.../spi/ingestion/batch/IngestionJobLauncher.java | 1 +
.../apache/pinot/spi/plugin/PluginClassLoader.java | 15 +-
.../org/apache/pinot/spi/plugin/PluginManager.java | 7 +-
...ionJobSpec.yaml => hadoopIngestionJobSpec.yaml} | 8 +-
36 files changed, 937 insertions(+), 505 deletions(-)
create mode 100644
pinot-plugins/pinot-batch-ingestion/pinot-batch-ingestion-common/src/main/java/org/apache/pinot/plugin/ingestion/batch/common/SegmentGenerationUtils.java
copy {pinot-spi =>
pinot-plugins/pinot-batch-ingestion/pinot-batch-ingestion-hadoop}/pom.xml (59%)
create mode 100644
pinot-plugins/pinot-batch-ingestion/pinot-batch-ingestion-hadoop/src/main/java/org/apache/pinot/plugin/ingestion/batch/hadoop/HadoopSegmentCreationMapper.java
create mode 100644
pinot-plugins/pinot-batch-ingestion/pinot-batch-ingestion-hadoop/src/main/java/org/apache/pinot/plugin/ingestion/batch/hadoop/HadoopSegmentGenerationJobRunner.java
copy
pinot-plugins/pinot-batch-ingestion/{pinot-batch-ingestion-standalone/src/main/java/org/apache/pinot/plugin/ingestion/batch/standalone/SegmentTarPushJobRunner.java
=>
pinot-batch-ingestion-hadoop/src/main/java/org/apache/pinot/plugin/ingestion/batch/hadoop/HadoopSegmentTarPushJobRunner.java}
(85%)
copy
pinot-plugins/pinot-batch-ingestion/{pinot-batch-ingestion-standalone/src/main/java/org/apache/pinot/plugin/ingestion/batch/standalone/SegmentUriPushJobRunner.java
=>
pinot-batch-ingestion-hadoop/src/main/java/org/apache/pinot/plugin/ingestion/batch/hadoop/HadoopSegmentUriPushJobRunner.java}
(89%)
copy pinot-plugins/pinot-batch-ingestion/{pinot-batch-ingestion-spark =>
pinot-batch-ingestion-hadoop}/src/main/resources/segmentCreationAndTarPushJobSpec.yaml
(88%)
copy pinot-plugins/pinot-batch-ingestion/{pinot-batch-ingestion-standalone =>
pinot-batch-ingestion-hadoop}/src/main/resources/segmentCreationAndUriPushJobSpec.yaml
(89%)
copy pinot-plugins/pinot-batch-ingestion/{pinot-batch-ingestion-standalone =>
pinot-batch-ingestion-hadoop}/src/main/resources/segmentCreationJobSpec.yaml
(90%)
copy pinot-plugins/pinot-batch-ingestion/{pinot-batch-ingestion-standalone =>
pinot-batch-ingestion-hadoop}/src/main/resources/segmentTarPushJobSpec.yaml
(88%)
copy pinot-plugins/pinot-batch-ingestion/{pinot-batch-ingestion-spark =>
pinot-batch-ingestion-hadoop}/src/main/resources/segmentUriPushJobSpec.yaml
(89%)
copy
pinot-tools/src/main/resources/examples/batch/airlineStats/{sparkIngestionJobSpec.yaml
=> hadoopIngestionJobSpec.yaml} (97%)
-
To unsubscri
[incubator-pinot] 01/01: Make sql as default query format.
This is an automated email from the ASF dual-hosted git repository.
xiangfu pushed a commit to branch switch-ui-to-sql
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
commit 36303e5056fd340772e03940d2dc190a2515f193
Author: Xiang Fu
AuthorDate: Fri Jan 17 04:17:12 2020 -0800
Make sql as default query format.
---
pinot-controller/src/main/resources/static/js/init.js | 14 ++
.../src/main/resources/static/query/index.html | 8
pinot-controller/src/main/resources/webapp/index.html | 8
pinot-controller/src/main/resources/webapp/js/init.js | 8
4 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/pinot-controller/src/main/resources/static/js/init.js
b/pinot-controller/src/main/resources/static/js/init.js
index daddfe3..374535b 100644
--- a/pinot-controller/src/main/resources/static/js/init.js
+++ b/pinot-controller/src/main/resources/static/js/init.js
@@ -67,13 +67,13 @@ $(document).ready(function() {
// execute query and draw the results
var query = EDITOR.getValue().trim();
var traceEnabled = document.getElementById('trace-enabled').checked;
-var groupByModeSQL = document.getElementById('group-by-mode-sql').checked;
-var responseFormatSQL =
document.getElementById('response-format-sql').checked;
+var groupByModePQL = document.getElementById('group-by-mode-pql').checked;
+var responseFormatPQL =
document.getElementById('response-format-pql').checked;
var queryOptions = undefined;
-if (groupByModeSQL === true) {
+if (groupByModePQL !== true) {
queryOptions = "groupByMode=sql";
}
-if (responseFormatSQL === true) {
+if (responseFormatPQL !== true) {
if (queryOptions === undefined) {
queryOptions = "responseFormat=sql";
} else {
@@ -130,6 +130,12 @@ $(document).ready(function() {
row.push(aggregationGroup.value);
return row;
});
+} else if (queryResponse["resultTable"] &&
queryResponse.resultTable["rows"]
+&& queryResponse.resultTable.rows.length > 0) {
+ columnList = _.map(queryResponse.resultTable.dataSchema.columnNames,
function (columnName) {
+return {"title": columnName};
+ });
+ dataArray = queryResponse.resultTable.rows;
}
}
diff --git a/pinot-controller/src/main/resources/static/query/index.html
b/pinot-controller/src/main/resources/static/query/index.html
index 0e43c56..00ce760 100644
--- a/pinot-controller/src/main/resources/static/query/index.html
+++ b/pinot-controller/src/main/resources/static/query/index.html
@@ -79,11 +79,11 @@
Tracing
-
- Group By
Mode : SQL
+
+ Group By
Mode : PQL
-
- Query
Response Format : SQL
+
+ Query
Response Format : PQL
Run Query
diff --git a/pinot-controller/src/main/resources/webapp/index.html
b/pinot-controller/src/main/resources/webapp/index.html
index 3662d0f..e5025b9 100644
--- a/pinot-controller/src/main/resources/webapp/index.html
+++ b/pinot-controller/src/main/resources/webapp/index.html
@@ -75,11 +75,11 @@
Tracing
-
- Group By
Mode : SQL
+
+ Group By
Mode : PQL
-
- Query
Response Format : SQL
+
+ Query
Response Format : PQL
Run Query
diff --git a/pinot-controller/src/main/resources/webapp/js/init.js
b/pinot-controller/src/main/resources/webapp/js/init.js
index 6775aaa..d397b4a 100644
--- a/pinot-controller/src/main/resources/webapp/js/init.js
+++ b/pinot-controller/src/main/resources/webapp/js/init.js
@@ -65,13 +65,13 @@ $(document).ready(function() {
// execute query and draw the results
var query = EDITOR.getValue().trim();
var traceEnabled = document.getElementById('trace-enabled').checked;
-var groupByModeSQL = document.getElementById('group-by-mode-sql').checked;
-var responseFormatSQL =
document.getElementById('response-format-sql').checked;
+var groupByModePQL = document.getElementById('group-by-mode-pql').checked;
+var responseFormatPQL =
document.getElementById('response-format-pql').checked;
var queryOptions = undefined;
-if (groupByModeSQL === true) {
+if (groupByModePQL !== true) {
queryOptions = "groupByMode=sql";
}
-if (responseFormatSQL === true) {
+if (responseFormatPQL !== true) {
if (queryOptions === undefined) {
queryOptions = "responseFormat=sql";
} else {
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch switch-ui-to-sql created (now 36303e5)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch switch-ui-to-sql in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. at 36303e5 Make sql as default query format. This branch includes the following new commits: new 36303e5 Make sql as default query format. The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 opened a new pull request #4994: Make sql as default query format.
fx19880617 opened a new pull request #4994: Make sql as default query format. URL: https://github.com/apache/incubator-pinot/pull/4994 - Switch UI query format from pql to sql. - Fix data table presentation for sql response 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io commented on issue #4994: Make sql as default query format.
codecov-io commented on issue #4994: Make sql as default query format. URL: https://github.com/apache/incubator-pinot/pull/4994#issuecomment-575616116 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4994?src=pr&el=h1) Report > Merging [#4994](https://codecov.io/gh/apache/incubator-pinot/pull/4994?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/f990656ee442973d311157bfca993db6d8218874?src=pr&el=desc) will **decrease** coverage by `0.07%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4994?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#4994 +/- ## - Coverage 57.26% 57.19% -0.08% Complexity 12 12 Files 1177 1177 Lines 6245562455 Branches 9174 9174 - Hits 3576535719 -46 - Misses2402024050 +30 - Partials 2670 2686 +16 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4994?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `50% <0%> (-25%)` | `0% <0%> (ø)` | | | [...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `47.56% <0%> (-8.54%)` | `0% <0%> (ø)` | | | [...pache/pinot/core/util/SortedRangeIntersection.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1NvcnRlZFJhbmdlSW50ZXJzZWN0aW9uLmphdmE=) | `83.82% <0%> (-7.36%)` | `0% <0%> (ø)` | | | [...impl/dictionary/DoubleOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT25IZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `37.8% <0%> (-7.32%)` | `0% <0%> (ø)` | | | [.../impl/dictionary/FloatOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPbkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh) | `54.87% <0%> (-6.1%)` | `0% <0%> (ø)` | | | [...ore/startree/v2/builder/BaseSingleTreeBuilder.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS92Mi9idWlsZGVyL0Jhc2VTaW5nbGVUcmVlQnVpbGRlci5qYXZh) | `90.52% <0%> (-4.74%)` | `0% <0%> (ø)` | | | [...troller/helix/core/retention/RetentionManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JldGVudGlvbi9SZXRlbnRpb25NYW5hZ2VyLmphdmE=) | `75% <0%> (-4.17%)` | `0% <0%> (ø)` | | | [.../broker/routing/HelixExternalViewBasedRouting.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9IZWxpeEV4dGVybmFsVmlld0Jhc2VkUm91dGluZy5qYXZh) | `84.66% <0%> (-3.07%)` | `0% <0%> (ø)` | | | [...org/apache/pinot/client/DynamicBrokerSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0R5bmFtaWNCcm9rZXJTZWxlY3Rvci5qYXZh) | `69.69% <0%> (-3.04%)` | `0% <0%> (ø)` | | | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `47.61% <0%> (-2.39%)` | `0% <0%> (ø)` | | | ... and [17 more](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4994?src=pr&el=continue).
[GitHub] [incubator-pinot] npawar commented on a change in pull request #4994: Make sql as default query format.
npawar commented on a change in pull request #4994: Make sql as default query
format.
URL: https://github.com/apache/incubator-pinot/pull/4994#discussion_r368041313
##
File path: pinot-controller/src/main/resources/static/js/init.js
##
@@ -67,13 +67,13 @@ $(document).ready(function() {
// execute query and draw the results
var query = EDITOR.getValue().trim();
var traceEnabled = document.getElementById('trace-enabled').checked;
-var groupByModeSQL = document.getElementById('group-by-mode-sql').checked;
-var responseFormatSQL =
document.getElementById('response-format-sql').checked;
+var groupByModePQL = document.getElementById('group-by-mode-pql').checked;
+var responseFormatPQL =
document.getElementById('response-format-pql').checked;
var queryOptions = undefined;
-if (groupByModeSQL === true) {
+if (groupByModePQL !== true) {
queryOptions = "groupByMode=sql";
}
-if (responseFormatSQL === true) {
+if (responseFormatPQL !== true) {
if (queryOptions === undefined) {
Review comment:
Is the goal to invoke the new sql endpoint, or just set these options? If
it's the former, we need a new endpoint on the controller "/sql", similar to
"pql" which is in PqlQueryResource. Inside executeQuery method (called on line
83) we need to invoke "sql" endpoint accordingly.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on a change in pull request #4994: Make sql as default query format.
npawar commented on a change in pull request #4994: Make sql as default query format. URL: https://github.com/apache/incubator-pinot/pull/4994#discussion_r368043323 ## File path: pinot-controller/src/main/resources/static/query/index.html ## @@ -79,11 +79,11 @@ Pinot Data Explorer Tracing - Review comment: We need only 1 checkbox now for "PQL", which will go to the "/pql" endpoint. By default we go to "sql" endpoint 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on issue #4994: Make sql as default query format.
npawar commented on issue #4994: Make sql as default query format. URL: https://github.com/apache/incubator-pinot/pull/4994#issuecomment-575715354 > * Switch UI query format from pql to sql. > * Fix data table presentation for sql response I think what we need is: 1. New Controller endpoint `/sql`, similar to `/pql`, which sets the right query options (see new `/query/sql` endpoint in PinotCLientRequest) 2. Invoke `/sql` by default in query console. Have 1 checkbox, `PQL` to switch to `pql` endpoint. Any specific reason we need to continue to expose granular options groupByMode and responseFormat to the user? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #4993: Support Text Search (both offline and realtime)
siddharthteotia commented on a change in pull request #4993: Support Text
Search (both offline and realtime)
URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r368072349
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/indexsegment/mutable/MutableSegmentImpl.java
##
@@ -281,10 +334,14 @@ public long getLatestIngestionTimestamp() {
* @param column column name
* @return true if column is no-dictionary, false if dictionary encoded
*/
- private boolean isNoDictionaryColumn(Set noDictionaryColumns,
Set invertedIndexColumns,
- FieldSpec fieldSpec, String column) {
-return noDictionaryColumns.contains(column) &&
fieldSpec.isSingleValueField()
-&& !invertedIndexColumns.contains(column);
+ private boolean isNoDictionaryColumn(
Review comment:
Done -- I explicitly added the arguments in separate line as without
indentation, functions with multiple arguments look a bit ugly. Reverted
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #4993: Support Text Search (both offline and realtime)
siddharthteotia commented on a change in pull request #4993: Support Text
Search (both offline and realtime)
URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r368104449
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/FilterOperatorUtils.java
##
@@ -148,8 +153,11 @@ int getPriority(BaseFilterOperator filterOperator) {
if (filterOperator instanceof OrFilterOperator) {
return 3;
}
+if (filterOperator instanceof TextMatchFilterOperator) {
Review comment:
It is going to happen like that only since TextMatch will return
BitMapDocIdSet and the AndBlockDocIdSet is intelligent about keeping bitmaps at
the top and append the ones in remaining category (child AND/OR) and the scan
based category (at the end.
I have adjusted the priority to 2. So now the order is
0 - Sort based inverted index
1 - bitmap inverted index
2 - text match
3 - AND
4 - OR
5 - SCAN
10 - Regex
Since 1 and 2 both return bitmaps, AndBlockDocIdSet will intersect them
upfront before creating the AndDocIdIterator -- this will take care of multiple
text_match() predicates or text_match() along with a predicate using native
Pinot inverted index.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] kishoreg commented on a change in pull request #4993: Support Text Search (both offline and realtime)
kishoreg commented on a change in pull request #4993: Support Text Search (both
offline and realtime)
URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r368117605
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/TextMatchFilterOperator.java
##
@@ -0,0 +1,67 @@
+/**
+ * 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.filter;
+
+import com.google.common.base.Preconditions;
+import org.apache.pinot.core.common.DataSource;
+import org.apache.pinot.core.common.Predicate;
+import org.apache.pinot.core.operator.blocks.FilterBlock;
+import org.apache.pinot.core.operator.docidsets.LuceneIndexDocIdSet;
+import org.apache.pinot.core.operator.filter.predicate.PredicateEvaluator;
+import
org.apache.pinot.core.operator.filter.predicate.TextMatchPredicateEvaluatorFactory;
+import org.apache.pinot.core.segment.index.readers.InvertedIndexReader;
+import org.apache.pinot.core.segment.index.readers.text.LuceneTextIndexReader;
+
+
+/**
+ * Filter operator for supporting the execution of text search
+ * queries: WHERE TEXT_MATCH(column_name, query_string)
+ */
+public class TextMatchFilterOperator extends BaseFilterOperator {
+ private static final String OPERATOR_NAME = "TextMatchFilterOperator";
+
+ private final Predicate _predicate;
+ private final DataSource _dataSource;
+ private final int _startDocId;
+ private final int _endDocId;
+
+ public TextMatchFilterOperator(PredicateEvaluator predicateEvaluator,
DataSource dataSource, int startDocId, int endDocId) {
+Preconditions.checkArgument(predicateEvaluator instanceof
TextMatchPredicateEvaluatorFactory.RawValueBasedTextMatchPredicateEvaluator &&
+!predicateEvaluator.isAlwaysTrue() && !predicateEvaluator.isAlwaysFalse());
+
TextMatchPredicateEvaluatorFactory.RawValueBasedTextMatchPredicateEvaluator
evaluator =
(TextMatchPredicateEvaluatorFactory.RawValueBasedTextMatchPredicateEvaluator)predicateEvaluator;
+_predicate = evaluator.getPredicate();
+_dataSource = dataSource;
+_startDocId = startDocId;
+_endDocId = endDocId;
+ }
+
+ @Override
+ protected FilterBlock getNextBlock() {
+InvertedIndexReader textIndexReader = _dataSource.getInvertedIndex();
+Preconditions.checkNotNull(textIndexReader, "Error: expecting non-null
text index");
+LuceneTextIndexReader.LuceneSearchResult luceneSearchResult =
(LuceneTextIndexReader.LuceneSearchResult)textIndexReader.getDocIds(_predicate);
+// TODO: For the next iteration of this feature, consider using
BitMapDocIdSet (and BitMapDocIdIterator)
Review comment:
Thanks
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #4993: Support Text column type in Pinot (both offline and realtime)
siddharthteotia commented on a change in pull request #4993: Support Text
column type in Pinot (both offline and realtime)
URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r368135253
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/LuceneIndexScanDocIdIterator.java
##
@@ -0,0 +1,90 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.lucene.document.Document;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.pinot.core.common.Constants;
+import org.apache.pinot.core.segment.creator.impl.V1Constants;
+import
org.apache.pinot.core.segment.index.readers.text.LuceneIndexSearcherReferenceManager;
+import org.apache.pinot.core.segment.index.readers.text.LuceneTextIndexReader;
+import org.roaringbitmap.IntIterator;
+
+
+public class LuceneIndexScanDocIdIterator implements IndexBasedDocIdIterator {
+
+ private int _currentDocId = -1;
+ private int _startDocId;
+ private int _endDocId;
+ private final IntIterator _docIDIterator;
+ private final IndexSearcher _indexSearcher;
+ private final LuceneIndexSearcherReferenceManager _searcherReferenceManager;
+ private final int _numDocs;
+
+ public LuceneIndexScanDocIdIterator(LuceneTextIndexReader.LuceneSearchResult
luceneSearchResult) {
+_docIDIterator = luceneSearchResult.getDocIDs().getIntIterator();
Review comment:
Discussed offline. It is better to iterate over the lucene doc id set
upfront and build the bitmap with actual pinot doc ids and then have rest of
the filter processing work off that bimap
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #4993: Support Text column type in Pinot (both offline and realtime)
siddharthteotia commented on a change in pull request #4993: Support Text column type in Pinot (both offline and realtime) URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r368134995 ## File path: pinot-core/pom.xml ## @@ -198,5 +198,20 @@ test-jar test + + org.apache.lucene + lucene-core + 8.2.0 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #4993: Support Text column type in Pinot (both offline and realtime)
siddharthteotia commented on a change in pull request #4993: Support Text
column type in Pinot (both offline and realtime)
URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r368135722
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/TextMatchFilterOperator.java
##
@@ -0,0 +1,67 @@
+/**
+ * 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.filter;
+
+import com.google.common.base.Preconditions;
+import org.apache.pinot.core.common.DataSource;
+import org.apache.pinot.core.common.Predicate;
+import org.apache.pinot.core.operator.blocks.FilterBlock;
+import org.apache.pinot.core.operator.docidsets.LuceneIndexDocIdSet;
+import org.apache.pinot.core.operator.filter.predicate.PredicateEvaluator;
+import
org.apache.pinot.core.operator.filter.predicate.TextMatchPredicateEvaluatorFactory;
+import org.apache.pinot.core.segment.index.readers.InvertedIndexReader;
+import org.apache.pinot.core.segment.index.readers.text.LuceneTextIndexReader;
+
+
+/**
+ * Filter operator for supporting the execution of text search
+ * queries: WHERE TEXT_MATCH(column_name, query_string)
+ */
+public class TextMatchFilterOperator extends BaseFilterOperator {
Review comment:
Done -- the operator stays the same (TextMatchFilterOperator) but it now
outputs a BitMapDocIdSet
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #4993: Support Text column type in Pinot (both offline and realtime)
siddharthteotia commented on a change in pull request #4993: Support Text
column type in Pinot (both offline and realtime)
URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r368135838
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/LuceneIndexDocIdSet.java
##
@@ -0,0 +1,84 @@
+/**
+ * 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.docidsets;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import
org.apache.pinot.core.operator.dociditerators.LuceneIndexScanDocIdIterator;
+import org.apache.pinot.core.segment.index.readers.text.LuceneTextIndexReader;
+
+
+public class LuceneIndexDocIdSet implements FilterBlockDocIdSet {
Review comment:
Removed this class after latest discussion
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #4993: Support Text column type in Pinot (both offline and realtime)
siddharthteotia commented on a change in pull request #4993: Support Text
column type in Pinot (both offline and realtime)
URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r368135253
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/LuceneIndexScanDocIdIterator.java
##
@@ -0,0 +1,90 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.lucene.document.Document;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.pinot.core.common.Constants;
+import org.apache.pinot.core.segment.creator.impl.V1Constants;
+import
org.apache.pinot.core.segment.index.readers.text.LuceneIndexSearcherReferenceManager;
+import org.apache.pinot.core.segment.index.readers.text.LuceneTextIndexReader;
+import org.roaringbitmap.IntIterator;
+
+
+public class LuceneIndexScanDocIdIterator implements IndexBasedDocIdIterator {
+
+ private int _currentDocId = -1;
+ private int _startDocId;
+ private int _endDocId;
+ private final IntIterator _docIDIterator;
+ private final IndexSearcher _indexSearcher;
+ private final LuceneIndexSearcherReferenceManager _searcherReferenceManager;
+ private final int _numDocs;
+
+ public LuceneIndexScanDocIdIterator(LuceneTextIndexReader.LuceneSearchResult
luceneSearchResult) {
+_docIDIterator = luceneSearchResult.getDocIDs().getIntIterator();
Review comment:
Discussed offline. It is better to iterate over the lucene doc id set
upfront and build the bitmap with actual pinot doc ids and then have rest of
the filter processing work off that bimap since then we can do fast
intersection for multi-predicate query which is either leveraging native Pinot
inverted index or another text_match predicate.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #4993: Support Text column type in Pinot (both offline and realtime)
siddharthteotia commented on a change in pull request #4993: Support Text
column type in Pinot (both offline and realtime)
URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r368136714
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/indexsegment/mutable/MutableSegmentImpl.java
##
@@ -418,28 +474,35 @@ private void addForwardIndex(GenericRow row, int docId,
Map dict
}
}
} else {
+// MV column: always dictionary encoded
int[] dictIds = (int[]) dictIdMap.get(column);
((FixedByteSingleColumnMultiValueReaderWriter)
_indexReaderWriterMap.get(column)).setIntArray(docId, dictIds);
}
}
}
- private void addInvertedIndex(int docId, Map dictIdMap) {
+ private void addInvertedIndex(GenericRow row, int docId, Map
dictIdMap) {
// Update inverted index at last
// NOTE: inverted index have to be updated at last because once it gets
updated, the latest record will become
// queryable
for (FieldSpec fieldSpec : _physicalFieldSpecs) {
String column = fieldSpec.getName();
- RealtimeInvertedIndexReader invertedIndex =
_invertedIndexMap.get(column);
+ InvertedIndexReader invertedIndex = _invertedIndexMap.get(column);
if (invertedIndex != null) {
-if (fieldSpec.isSingleValueField()) {
- invertedIndex.add(((Integer) dictIdMap.get(column)), docId);
+if (invertedIndex instanceof RealtimeLuceneTextIndexReader) {
Review comment:
Discussed offlineThe existing RealtimeInvertedIndexReader is actually
implemented as a reader and writer. A follow-up PR will come to cleanup this
interface and then we can avoid typecasting.
Right now typecasting is not that bad -- done only when realtime segment is
destroyed.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #4993: Support Text column type in Pinot (both offline and realtime)
siddharthteotia commented on a change in pull request #4993: Support Text
column type in Pinot (both offline and realtime)
URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r367800344
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/LuceneIndexScanDocIdIterator.java
##
@@ -0,0 +1,90 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.lucene.document.Document;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.pinot.core.common.Constants;
+import org.apache.pinot.core.segment.creator.impl.V1Constants;
+import
org.apache.pinot.core.segment.index.readers.text.LuceneIndexSearcherReferenceManager;
+import org.apache.pinot.core.segment.index.readers.text.LuceneTextIndexReader;
+import org.roaringbitmap.IntIterator;
+
+
+public class LuceneIndexScanDocIdIterator implements IndexBasedDocIdIterator {
+
+ private int _currentDocId = -1;
+ private int _startDocId;
+ private int _endDocId;
+ private final IntIterator _docIDIterator;
+ private final IndexSearcher _indexSearcher;
+ private final LuceneIndexSearcherReferenceManager _searcherReferenceManager;
+ private final int _numDocs;
+
+ public LuceneIndexScanDocIdIterator(LuceneTextIndexReader.LuceneSearchResult
luceneSearchResult) {
+_docIDIterator = luceneSearchResult.getDocIDs().getIntIterator();
Review comment:
When Lucene returns search result... we get the docIDs collected by the
collector in MutableRoaringBitmap.
These are essentially Lucene internal docIDs (during index building, lucene
creates separate index structures (it calls them segments) and they are merged
as per its internal merge criteria so we can't rely on its internal docIds). In
some cases, they can be same as Pinot docIDs but based on the index size, how
many index segments it creates and how it merges etc can vary so relying on
them permanently is very likely to break our code and they also advise against
it.
So the way to get the actual Pinot docIDs is to store them with each Lucene
document
Once the IndexSearcher finishes searching each document and all lucene
docIDs are collected by the collector in the bitmap, we can do
searcher.doc(lucene doc id), this will get the document and we can retrieve the
actual docID from the document.
Now why can't we do searcher.doc(lucene doc id) as part of collector? It
suffers severe performance -- (something there documentation also says not to
do this while the collector is being invoked).
So that's why this bitmap with lucene docids is then iterated by
LuceneIndexDocIdIterator and for each lucene doc ID, we get the actual docID
and this is what is returned by the iterator for rest of the filter processing
to continue. Once we have iterated completely, we also release the searcher via
reference manager (NO-OP for offline search).
Now if we were to use BitMapDocIdSet -- which directly works against the
bitmap of docIDs, we have two options
(1) Change the implementation of BitmapDocIdSet to take LuceneSearchResult
in the constructor and do all of this there and in BitMapDocIdIterator
OR
(2) Once the call to textIndexReader.getDocIds(predicate) comes back into
TextMatchFilterOperator, we iterate over the lucene docIDs there itself and get
the Pinot docIDs (as described above) and collect them in a new bitmap. Pass
this bitmap over to BitmapDocIdSet. Everything will work correctly but then we
have to construct another bitmap which will impact performance and memory
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #4993: Support Text column type in Pinot (both offline and realtime)
siddharthteotia commented on a change in pull request #4993: Support Text
column type in Pinot (both offline and realtime)
URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r368141436
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/LuceneIndexScanDocIdIterator.java
##
@@ -0,0 +1,90 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.lucene.document.Document;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.pinot.core.common.Constants;
+import org.apache.pinot.core.segment.creator.impl.V1Constants;
+import
org.apache.pinot.core.segment.index.readers.text.LuceneIndexSearcherReferenceManager;
+import org.apache.pinot.core.segment.index.readers.text.LuceneTextIndexReader;
+import org.roaringbitmap.IntIterator;
+
+
+public class LuceneIndexScanDocIdIterator implements IndexBasedDocIdIterator {
+
+ private int _currentDocId = -1;
+ private int _startDocId;
+ private int _endDocId;
+ private final IntIterator _docIDIterator;
+ private final IndexSearcher _indexSearcher;
+ private final LuceneIndexSearcherReferenceManager _searcherReferenceManager;
+ private final int _numDocs;
+
+ public LuceneIndexScanDocIdIterator(LuceneTextIndexReader.LuceneSearchResult
luceneSearchResult) {
+_docIDIterator = luceneSearchResult.getDocIDs().getIntIterator();
Review comment:
As a follow-up we can optimize this path to see if a construction of new
bitmap is needed or not
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #4993: Support Text column type in Pinot (both offline and realtime)
siddharthteotia commented on a change in pull request #4993: Support Text
column type in Pinot (both offline and realtime)
URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r368136714
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/indexsegment/mutable/MutableSegmentImpl.java
##
@@ -418,28 +474,35 @@ private void addForwardIndex(GenericRow row, int docId,
Map dict
}
}
} else {
+// MV column: always dictionary encoded
int[] dictIds = (int[]) dictIdMap.get(column);
((FixedByteSingleColumnMultiValueReaderWriter)
_indexReaderWriterMap.get(column)).setIntArray(docId, dictIds);
}
}
}
- private void addInvertedIndex(int docId, Map dictIdMap) {
+ private void addInvertedIndex(GenericRow row, int docId, Map
dictIdMap) {
// Update inverted index at last
// NOTE: inverted index have to be updated at last because once it gets
updated, the latest record will become
// queryable
for (FieldSpec fieldSpec : _physicalFieldSpecs) {
String column = fieldSpec.getName();
- RealtimeInvertedIndexReader invertedIndex =
_invertedIndexMap.get(column);
+ InvertedIndexReader invertedIndex = _invertedIndexMap.get(column);
if (invertedIndex != null) {
-if (fieldSpec.isSingleValueField()) {
- invertedIndex.add(((Integer) dictIdMap.get(column)), docId);
+if (invertedIndex instanceof RealtimeLuceneTextIndexReader) {
Review comment:
Discussed offlineThe existing RealtimeInvertedIndexReader is actually
implemented as a reader and writer without a proper interface. A follow-up PR
will come to cleanup this interface and then we can avoid typecasting.
Right now typecasting is not that bad -- done only when realtime segment is
destroyed.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat opened a new pull request #4995: Decouple server instance id with hostname/port config.
chenboat opened a new pull request #4995: Decouple server instance id with hostname/port config. URL: https://github.com/apache/incubator-pinot/pull/4995 Currently Helix will derived hostname/port from the server instanceId string (as demonstrated in the test _testDisableLogicalServerID()_) if the instance id is set in server config. This PR decouples the configuration of server instance id and hostname/port. This allows Pinot user to set the following field independently: - Instance ID - server_netty_host - server_netty_port This is done by overwriting the Helix zk data after server connects to the zk cluster. It is hidden behind a flag now so the current use cases will not be affected. Also added an integration test to test different configuration choices for servers. @Jackie-Jiang @icefury71 Issue link #4525 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch switch-ui-to-sql updated (36303e5 -> e9f315e)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch switch-ui-to-sql in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard 36303e5 Make sql as default query format. add e9f315e Make sql as default query format. This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (36303e5) \ N -- N -- N refs/heads/switch-ui-to-sql (e9f315e) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new revisions were added by this update. Summary of changes: .../request/transform/TransformExpressionTree.java | 5 ++ .../src/main/resources/static/js/init.js | 95 ++ .../src/main/resources/static/query/index.html | 7 +- .../src/main/resources/webapp/index.html | 7 +- .../src/main/resources/webapp/js/init.js | 22 ++--- 5 files changed, 61 insertions(+), 75 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] icefury71 commented on a change in pull request #4995: Decouple server instance id with hostname/port config.
icefury71 commented on a change in pull request #4995: Decouple server instance
id with hostname/port config.
URL: https://github.com/apache/incubator-pinot/pull/4995#discussion_r368160212
##
File path:
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ServerStarterIntegrationTest.java
##
@@ -0,0 +1,179 @@
+/**
+ * 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.integration.tests;
+
+
+import org.apache.commons.configuration.Configuration;
+import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.helix.HelixManager;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.model.InstanceConfig;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.common.utils.ZkStarter;
+import org.apache.pinot.controller.helix.ControllerTest;
+import org.apache.pinot.server.starter.helix.HelixServerStarter;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.KEY_OF_SERVER_NETTY_HOST;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.KEY_OF_SERVER_NETTY_PORT;
+import static
org.apache.pinot.common.utils.CommonConstants.Server.CONFIG_OF_INSTANCE_ID;
+import static
org.apache.pinot.common.utils.CommonConstants.Server.CONFIG_OF_USE_LOGICAL_INSTANCE_ID;
+import static org.testng.Assert.assertEquals;
+
+
+public class ServerStarterIntegrationTest extends ControllerTest {
+ public static final String DEFAULT_SERVER_ID = "Server_127.0.0.1_8098";
+ public static final String SERVER1 = "Server1";
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+startZk();
+startController();
+ }
+
+ @AfterClass
+ public void tearDown()
+ throws Exception {
+stopController();
+stopZk();
+ }
+
+ @Test
+ public void testWithNoInstanceIdNoHostnamePort()
+ throws Exception {
+// Test the behavior when no instance id nor hostname/port is specified in
server conf.
+Configuration serverConf = new PropertiesConfiguration();
+// Start the server
+HelixServerStarter helixServerStarter =
+new HelixServerStarter(getHelixClusterName(),
ZkStarter.DEFAULT_ZK_STR, serverConf);
+
+// Verify the serverId, host and port are set correctly in Zk.
+HelixManager helixManager = helixServerStarter.getHelixManager();
+PropertyKey.Builder keyBuilder =
helixManager.getHelixDataAccessor().keyBuilder();
+InstanceConfig config =
helixManager.getHelixDataAccessor().getProperty(keyBuilder.instanceConfig(DEFAULT_SERVER_ID));
+helixServerStarter.stop();
+
+assertEquals(config.getInstanceName(), DEFAULT_SERVER_ID);
+// By default (auto joined instances), server instance name is of format:
{@code Server__}, e.g.
+// {@code Server_localhost_12345}, hostname is of format: {@code
Server_}, e.g. {@code Server_localhost}.
+// More details refer to the class ServerInstance.
+assertEquals(config.getHostName(), "Server_127.0.0.1");
+assertEquals(config.getPort(), "8098");
+ }
+
+ @Test
+ public void testWithNoInstanceIdButWithHostnamePort()
+ throws Exception {
+// Test the behavior when no instance id specified but hostname/port is
specified in server conf.
+Configuration serverConf = new PropertiesConfiguration();
+serverConf.setProperty(KEY_OF_SERVER_NETTY_HOST, "host1");
+serverConf.setProperty(KEY_OF_SERVER_NETTY_PORT, 10001);
+// Start the server
+HelixServerStarter helixServerStarter =
Review comment:
Following code looks like its repeated in every test. Consider refactoring ?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands,
[GitHub] [incubator-pinot] icefury71 commented on a change in pull request #4995: Decouple server instance id with hostname/port config.
icefury71 commented on a change in pull request #4995: Decouple server instance
id with hostname/port config.
URL: https://github.com/apache/incubator-pinot/pull/4995#discussion_r368160531
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java
##
@@ -192,6 +192,9 @@ public ServerType getServerType() {
public static final String CONFIG_OF_REQUEST_HANDLER_FACTORY_CLASS =
"pinot.server.requestHandlerFactory.class";
public static final String CONFIG_OF_NETTY_PORT =
"pinot.server.netty.port";
public static final String CONFIG_OF_ADMIN_API_PORT =
"pinot.server.adminapi.port";
+// A logical instance id is one which does not contain server host name
and/or port info. E.g., server1.
+// It is by default disabled.
+public static final String CONFIG_OF_USE_LOGICAL_INSTANCE_ID =
"pinot.server.logical.instance.id.enabled";
Review comment:
Why do we need this ? I think we will always use the Zk info to look up host
and port (whether logical id is enabled or not).
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] icefury71 commented on a change in pull request #4995: Decouple server instance id with hostname/port config.
icefury71 commented on a change in pull request #4995: Decouple server instance
id with hostname/port config.
URL: https://github.com/apache/incubator-pinot/pull/4995#discussion_r368160793
##
File path:
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixServerStarter.java
##
@@ -145,6 +146,11 @@ public HelixServerStarter(String helixClusterName, String
zkServer, Configuratio
LOGGER.info("Connecting Helix manager");
_helixManager.connect();
+// Overwrite the server netty host and port.
+if (_serverConf.getBoolean(CONFIG_OF_USE_LOGICAL_INSTANCE_ID, false)) {
Review comment:
You could also simply look up if the server config has instance id set ?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4994: Make sql as default query format.
codecov-io edited a comment on issue #4994: Make sql as default query format. URL: https://github.com/apache/incubator-pinot/pull/4994#issuecomment-575616116 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4994?src=pr&el=h1) Report > Merging [#4994](https://codecov.io/gh/apache/incubator-pinot/pull/4994?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/f990656ee442973d311157bfca993db6d8218874?src=pr&el=desc) will **decrease** coverage by `0.04%`. > The diff coverage is `0%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4994?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#4994 +/- ## - Coverage 57.26% 57.21% -0.05% Complexity 12 12 Files 1177 1177 Lines 6245562459 +4 Branches 9174 9175 +1 - Hits 3576535739 -26 - Misses2402024047 +27 - Partials 2670 2673 +3 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4994?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...mon/request/transform/TransformExpressionTree.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC90cmFuc2Zvcm0vVHJhbnNmb3JtRXhwcmVzc2lvblRyZWUuamF2YQ==) | `74.24% <0%> (-4.79%)` | `0 <0> (ø)` | | | [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `50% <0%> (-25%)` | `0% <0%> (ø)` | | | [...e/operator/dociditerators/SortedDocIdIterator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9Tb3J0ZWREb2NJZEl0ZXJhdG9yLmphdmE=) | `47.22% <0%> (-8.34%)` | `0% <0%> (ø)` | | | [...mpl/dictionary/DoubleOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT2ZmSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `43.01% <0%> (-6.46%)` | `0% <0%> (ø)` | | | [.../impl/dictionary/LongOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09mZkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh) | `53.76% <0%> (-6.46%)` | `0% <0%> (ø)` | | | [.../impl/dictionary/FloatOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPbkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh) | `54.87% <0%> (-6.1%)` | `0% <0%> (ø)` | | | [...troller/helix/core/retention/RetentionManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JldGVudGlvbi9SZXRlbnRpb25NYW5hZ2VyLmphdmE=) | `75% <0%> (-4.17%)` | `0% <0%> (ø)` | | | [.../pinot/core/operator/docidsets/BitmapDocIdSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZHNldHMvQml0bWFwRG9jSWRTZXQuamF2YQ==) | `81.25% <0%> (-3.13%)` | `0% <0%> (ø)` | | | [...not/broker/broker/helix/ClusterChangeMediator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0NsdXN0ZXJDaGFuZ2VNZWRpYXRvci5qYXZh) | `74.72% <0%> (-2.2%)` | `0% <0%> (ø)` | | | [...g/apache/pinot/common/utils/helix/HelixHelper.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvaGVsaXgvSGVsaXhIZWxwZXIuamF2YQ==) | `50.89% <0%> (-1.8%)` | `0% <0%> (ø)` | | | ... and [13 more](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4994?src=pr&el=con
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4995: Decouple server instance id with hostname/port config.
chenboat commented on a change in pull request #4995: Decouple server instance
id with hostname/port config.
URL: https://github.com/apache/incubator-pinot/pull/4995#discussion_r368170412
##
File path:
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ServerStarterIntegrationTest.java
##
@@ -0,0 +1,179 @@
+/**
+ * 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.integration.tests;
+
+
+import org.apache.commons.configuration.Configuration;
+import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.helix.HelixManager;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.model.InstanceConfig;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.common.utils.ZkStarter;
+import org.apache.pinot.controller.helix.ControllerTest;
+import org.apache.pinot.server.starter.helix.HelixServerStarter;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.KEY_OF_SERVER_NETTY_HOST;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.KEY_OF_SERVER_NETTY_PORT;
+import static
org.apache.pinot.common.utils.CommonConstants.Server.CONFIG_OF_INSTANCE_ID;
+import static
org.apache.pinot.common.utils.CommonConstants.Server.CONFIG_OF_USE_LOGICAL_INSTANCE_ID;
+import static org.testng.Assert.assertEquals;
+
+
+public class ServerStarterIntegrationTest extends ControllerTest {
+ public static final String DEFAULT_SERVER_ID = "Server_127.0.0.1_8098";
+ public static final String SERVER1 = "Server1";
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+startZk();
+startController();
+ }
+
+ @AfterClass
+ public void tearDown()
+ throws Exception {
+stopController();
+stopZk();
+ }
+
+ @Test
+ public void testWithNoInstanceIdNoHostnamePort()
+ throws Exception {
+// Test the behavior when no instance id nor hostname/port is specified in
server conf.
+Configuration serverConf = new PropertiesConfiguration();
+// Start the server
+HelixServerStarter helixServerStarter =
+new HelixServerStarter(getHelixClusterName(),
ZkStarter.DEFAULT_ZK_STR, serverConf);
+
+// Verify the serverId, host and port are set correctly in Zk.
+HelixManager helixManager = helixServerStarter.getHelixManager();
+PropertyKey.Builder keyBuilder =
helixManager.getHelixDataAccessor().keyBuilder();
+InstanceConfig config =
helixManager.getHelixDataAccessor().getProperty(keyBuilder.instanceConfig(DEFAULT_SERVER_ID));
+helixServerStarter.stop();
+
+assertEquals(config.getInstanceName(), DEFAULT_SERVER_ID);
+// By default (auto joined instances), server instance name is of format:
{@code Server__}, e.g.
+// {@code Server_localhost_12345}, hostname is of format: {@code
Server_}, e.g. {@code Server_localhost}.
+// More details refer to the class ServerInstance.
+assertEquals(config.getHostName(), "Server_127.0.0.1");
+assertEquals(config.getPort(), "8098");
+ }
+
+ @Test
+ public void testWithNoInstanceIdButWithHostnamePort()
+ throws Exception {
+// Test the behavior when no instance id specified but hostname/port is
specified in server conf.
+Configuration serverConf = new PropertiesConfiguration();
+serverConf.setProperty(KEY_OF_SERVER_NETTY_HOST, "host1");
+serverConf.setProperty(KEY_OF_SERVER_NETTY_PORT, 10001);
+// Start the server
+HelixServerStarter helixServerStarter =
Review comment:
Code refactored to remove duplicate codes.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4995: Decouple server instance id with hostname/port config.
chenboat commented on a change in pull request #4995: Decouple server instance
id with hostname/port config.
URL: https://github.com/apache/incubator-pinot/pull/4995#discussion_r368172418
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java
##
@@ -192,6 +192,9 @@ public ServerType getServerType() {
public static final String CONFIG_OF_REQUEST_HANDLER_FACTORY_CLASS =
"pinot.server.requestHandlerFactory.class";
public static final String CONFIG_OF_NETTY_PORT =
"pinot.server.netty.port";
public static final String CONFIG_OF_ADMIN_API_PORT =
"pinot.server.adminapi.port";
+// A logical instance id is one which does not contain server host name
and/or port info. E.g., server1.
+// It is by default disabled.
+public static final String CONFIG_OF_USE_LOGICAL_INSTANCE_ID =
"pinot.server.logical.instance.id.enabled";
Review comment:
This config flag is used only for server start and mainly used for backward
compatibility. It controls if we want to overwrite the host/port info in zk
using those found server config. The current behavior is to extract hostname
from instance id if instance id is found in server config. We do not want to
break these use cases for now.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4995: Decouple server instance id with hostname/port config.
chenboat commented on a change in pull request #4995: Decouple server instance
id with hostname/port config.
URL: https://github.com/apache/incubator-pinot/pull/4995#discussion_r368172623
##
File path:
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixServerStarter.java
##
@@ -145,6 +146,11 @@ public HelixServerStarter(String helixClusterName, String
zkServer, Configuratio
LOGGER.info("Connecting Helix manager");
_helixManager.connect();
+// Overwrite the server netty host and port.
+if (_serverConf.getBoolean(CONFIG_OF_USE_LOGICAL_INSTANCE_ID, false)) {
Review comment:
Similar consideration as above.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #4993: Support Text column type in Pinot (both offline and realtime)
siddharthteotia commented on a change in pull request #4993: Support Text
column type in Pinot (both offline and realtime)
URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r368180500
##
File path: pinot-common/src/main/antlr4/org/apache/pinot/pql/parsers/PQL2.g4
##
@@ -95,6 +96,9 @@ betweenClause:
regexpLikeClause:
REGEXP_LIKE '(' expression ',' literal ')';
+textMatchClause:
Review comment:
We need to extend the Calcite grammar and parser to make its compiler
recognize TEXT_MATCH as a valid token. This way a SQL query on Pinot that has
text_match() predicate(s) and goes down the Calcite compiler path to build
PinotQuery will work perfectly
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #4993: Support Text column type in Pinot (both offline and realtime)
siddharthteotia commented on a change in pull request #4993: Support Text
column type in Pinot (both offline and realtime)
URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r368180538
##
File path: pinot-common/src/main/antlr4/org/apache/pinot/pql/parsers/PQL2.g4
##
@@ -95,6 +96,9 @@ betweenClause:
regexpLikeClause:
REGEXP_LIKE '(' expression ',' literal ')';
+textMatchClause:
Review comment:
Will do that as a follow-up
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #4993: Support Text column type in Pinot (both offline and realtime)
siddharthteotia commented on a change in pull request #4993: Support Text
column type in Pinot (both offline and realtime)
URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r368136714
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/indexsegment/mutable/MutableSegmentImpl.java
##
@@ -418,28 +474,35 @@ private void addForwardIndex(GenericRow row, int docId,
Map dict
}
}
} else {
+// MV column: always dictionary encoded
int[] dictIds = (int[]) dictIdMap.get(column);
((FixedByteSingleColumnMultiValueReaderWriter)
_indexReaderWriterMap.get(column)).setIntArray(docId, dictIds);
}
}
}
- private void addInvertedIndex(int docId, Map dictIdMap) {
+ private void addInvertedIndex(GenericRow row, int docId, Map
dictIdMap) {
// Update inverted index at last
// NOTE: inverted index have to be updated at last because once it gets
updated, the latest record will become
// queryable
for (FieldSpec fieldSpec : _physicalFieldSpecs) {
String column = fieldSpec.getName();
- RealtimeInvertedIndexReader invertedIndex =
_invertedIndexMap.get(column);
+ InvertedIndexReader invertedIndex = _invertedIndexMap.get(column);
if (invertedIndex != null) {
-if (fieldSpec.isSingleValueField()) {
- invertedIndex.add(((Integer) dictIdMap.get(column)), docId);
+if (invertedIndex instanceof RealtimeLuceneTextIndexReader) {
Review comment:
Discussed offlineThe existing RealtimeInvertedIndexReader is actually
implemented as a reader and writer without a proper interface. Since that is
orthogonal piece of work and requires cleanup of existing interfaces, a
follow-up PR will come to cleanup this interface and then we can avoid
typecasting.
Also, right now typecasting is not that bad -- done only when realtime
segment is destroyed.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #4993: Support Text column type in Pinot (both offline and realtime)
siddharthteotia commented on a change in pull request #4993: Support Text
column type in Pinot (both offline and realtime)
URL: https://github.com/apache/incubator-pinot/pull/4993#discussion_r368181968
##
File path:
pinot-common/src/main/java/org/apache/pinot/pql/parsers/pql2/ast/TextMatchPredicateAstNode.java
##
@@ -0,0 +1,82 @@
+/**
+ * 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.pql.parsers.pql2.ast;
+
+import com.google.common.base.Preconditions;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.FilterOperator;
+import org.apache.pinot.common.utils.StringUtil;
+import org.apache.pinot.common.utils.request.FilterQueryTree;
+import org.apache.pinot.common.utils.request.HavingQueryTree;
+import org.apache.pinot.common.utils.request.RequestUtils;
+import org.apache.pinot.pql.parsers.Pql2CompilationException;
+
+
+public class TextMatchPredicateAstNode extends PredicateAstNode {
+
+ private static final String SEPERATOR = "\t\t";
+
+ @Override
+ public void addChild(AstNode childNode) {
+if (childNode instanceof IdentifierAstNode) {
+ if (_identifier == null) {
+IdentifierAstNode node = (IdentifierAstNode) childNode;
+_identifier = node.getName();
+ } else {
+throw new Pql2CompilationException("TEXT_MATCH predicate has more than
one identifier.");
+ }
+} else if (childNode instanceof FunctionCallAstNode) {
+ throw new Pql2CompilationException("TEXT_MATCH is not supported with
function");
+} else {
+ super.addChild(childNode);
+}
+ }
+
+ @Override
+ public FilterQueryTree buildFilterQueryTree() {
+if (_identifier == null) {
+ throw new Pql2CompilationException("TEXT_MATCH predicate has no
identifier");
+}
+
+List children = getChildren();
+Preconditions.checkState(children != null && children.size() == 1);
+AstNode child = children.get(0);
+Preconditions.checkState(child instanceof StringLiteralAstNode);
+String expr = ((StringLiteralAstNode)child).getValueAsString();
+FilterOperator filterOperator = FilterOperator.TEXT_MATCH;
+List value = Collections.singletonList(expr);
+return new FilterQueryTree(_identifier, value, filterOperator, null);
+ }
+
+ @Override
+ public Expression buildFilterExpression() {
Review comment:
When we build PinotQuery by going down the PQL compiler path:
(1) Build AST.
(2) Build BrokerRequest by doing root.updateBrokerRequest().
(3) Build PinotQuery by doing astNode.updatePinotQuery() implemented by our
AST nodes. Internally the WhereAstNode calls the buildFilterExpression() method
implemented by each type of PredicateAstNode. FilterExpression is needed by
PinotQuery.
I have implemented this 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch switch-ui-to-sql updated (e9f315e -> f0576e5)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch switch-ui-to-sql in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard e9f315e Make sql as default query format. add f0576e5 Make sql as default query format. This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (e9f315e) \ N -- N -- N refs/heads/switch-ui-to-sql (f0576e5) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new revisions were added by this update. Summary of changes: pinot-controller/src/main/resources/static/js/init.js | 2 +- pinot-controller/src/main/resources/webapp/js/init.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #4994: Make sql as default query format.
fx19880617 commented on a change in pull request #4994: Make sql as default query format. URL: https://github.com/apache/incubator-pinot/pull/4994#discussion_r368186639 ## File path: pinot-controller/src/main/resources/static/query/index.html ## @@ -79,11 +79,11 @@ Pinot Data Explorer Tracing - Review comment: changed the ui to only show one check box of pql, which means to use pql syntax if checked. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on issue #4994: Make sql as default query format.
fx19880617 commented on issue #4994: Make sql as default query format. URL: https://github.com/apache/incubator-pinot/pull/4994#issuecomment-575843663 > > * Switch UI query format from pql to sql. > > * Fix data table presentation for sql response > > I think what we need is: > > 1. New Controller endpoint `/sql`, similar to `/pql`, which sets the right query options (see new `/query/sql` endpoint in PinotCLientRequest) > 2. Invoke `/sql` by default in query console. Have 1 checkbox, `PQL` to switch to `pql` endpoint. > > Any specific reason we need to continue to expose granular options groupByMode and responseFormat to the user? Here I'm still using pql endpoint as we haven't fully test out calcite sql parser. But the query response and group by behavior will follow sql standard. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4994: Make sql as default query format.
codecov-io edited a comment on issue #4994: Make sql as default query format. URL: https://github.com/apache/incubator-pinot/pull/4994#issuecomment-575616116 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4994?src=pr&el=h1) Report > Merging [#4994](https://codecov.io/gh/apache/incubator-pinot/pull/4994?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/f990656ee442973d311157bfca993db6d8218874?src=pr&el=desc) will **decrease** coverage by `<.01%`. > The diff coverage is `0%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4994?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#4994 +/- ## - Coverage 57.26% 57.26% -0.01% Complexity 12 12 Files 1177 1177 Lines 6245562459 +4 Branches 9174 9175 +1 + Hits 3576535766 +1 - Misses2402024024 +4 + Partials 2670 2669 -1 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4994?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...mon/request/transform/TransformExpressionTree.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC90cmFuc2Zvcm0vVHJhbnNmb3JtRXhwcmVzc2lvblRyZWUuamF2YQ==) | `74.24% <0%> (-4.79%)` | `0 <0> (ø)` | | | [...er/validation/BrokerResourceValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL0Jyb2tlclJlc291cmNlVmFsaWRhdGlvbk1hbmFnZXIuamF2YQ==) | `25% <0%> (-56.25%)` | `0% <0%> (ø)` | | | [...impl/dictionary/DoubleOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT25IZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `39.02% <0%> (-6.1%)` | `0% <0%> (ø)` | | | [...mpl/dictionary/DoubleOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT2ZmSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `44.08% <0%> (-5.38%)` | `0% <0%> (ø)` | | | [...troller/helix/core/retention/RetentionManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JldGVudGlvbi9SZXRlbnRpb25NYW5hZ2VyLmphdmE=) | `75% <0%> (-4.17%)` | `0% <0%> (ø)` | | | [...g/apache/pinot/common/utils/helix/HelixHelper.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvaGVsaXgvSGVsaXhIZWxwZXIuamF2YQ==) | `50.89% <0%> (-1.8%)` | `0% <0%> (ø)` | | | [...ata/manager/realtime/RealtimeTableDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVUYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `44.61% <0%> (-1.54%)` | `0% <0%> (ø)` | | | [.../java/org/apache/pinot/spi/data/TimeFieldSpec.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9UaW1lRmllbGRTcGVjLmphdmE=) | `92.59% <0%> (-1.24%)` | `0% <0%> (ø)` | | | [...ache/pinot/common/metadata/ZKMetadataProvider.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0YWRhdGEvWktNZXRhZGF0YVByb3ZpZGVyLmphdmE=) | `67.41% <0%> (-1.13%)` | `0% <0%> (ø)` | | | [...manager/realtime/LLRealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvTExSZWFsdGltZVNlZ21lbnREYXRhTWFuYWdlci5qYXZh) | `69.49% <0%> (-0.17%)` | `0% <0%> (ø)` | | | ... and [13 more](https://codecov.io/gh/apache/incubator-pinot/pull/4994/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4994?src=pr&el=continue). > **L
[GitHub] [incubator-pinot] vincentchenjl opened a new pull request #4996: [TE] change anomaly fetching to use intersection
vincentchenjl opened a new pull request #4996: [TE] change anomaly fetching to use intersection URL: https://github.com/apache/incubator-pinot/pull/4996 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] 01/01: Update dockerfile for pinot image and pinot-presto image
This is an automated email from the ASF dual-hosted git repository.
xiangfu pushed a commit to branch upgrade_docker_build_maven_to_3.6
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
commit d5fcaadc73762643def33a93a7be2e6455d53437
Author: Xiang Fu
AuthorDate: Fri Jan 17 23:05:55 2020 -0800
Update dockerfile for pinot image and pinot-presto image
---
docker/images/pinot-presto/Dockerfile | 3 ++-
docker/images/pinot/Dockerfile| 30 --
2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/docker/images/pinot-presto/Dockerfile
b/docker/images/pinot-presto/Dockerfile
index 23965f9..dce0f9d 100644
--- a/docker/images/pinot-presto/Dockerfile
+++ b/docker/images/pinot-presto/Dockerfile
@@ -30,7 +30,7 @@ ENV PRESTO_BUILD_DIR=/home/presto/build
# extra dependency for running launcher
RUN apt-get update && \
apt-get install -y --no-install-recommends \
-vim wget curl git maven && \
+vim wget curl git && \
rm -rf /var/lib/apt/lists/*
RUN groupadd -g 999 presto && \
@@ -40,6 +40,7 @@ USER presto
RUN git clone ${PRESTO_GIT_URL} ${PRESTO_BUILD_DIR} && \
cd ${PRESTO_BUILD_DIR} && \
git checkout ${PRESTO_BRANCH} && \
+echo
"distributionUrl=https://repo1.maven.org/maven2/org/apache/maven/apache-maven/3.6.3/apache-maven-3.6.3-bin.zip";
> .mvn/wrapper/maven-wrapper.properties && \
./mvnw clean install -DskipTests && \
mkdir -p ${PRESTO_HOME}/data && \
cp -r presto-server/target/presto-server-*/presto-server-*/*
${PRESTO_HOME} && \
diff --git a/docker/images/pinot/Dockerfile b/docker/images/pinot/Dockerfile
index 6354066..1fcabc5 100644
--- a/docker/images/pinot/Dockerfile
+++ b/docker/images/pinot/Dockerfile
@@ -30,15 +30,25 @@ ENV PINOT_BUILD_DIR=/opt/pinot-build
# extra dependency for running launcher
RUN apt-get update && \
-apt-get install -y --no-install-recommends vim wget curl git maven
automake bison flex g++ libboost-all-dev libevent-dev libssl-dev libtool make
pkg-config && \
-rm -rf /var/lib/apt/lists/* && \
-wget http://archive.apache.org/dist/thrift/0.12.0/thrift-0.12.0.tar.gz -O
/tmp/thrift-0.12.0.tar.gz && \
-tar xfz /tmp/thrift-0.12.0.tar.gz --directory /tmp && \
-base_dir=`pwd` && \
-cd /tmp/thrift-0.12.0 && \
-./configure --with-cpp=no --with-c_glib=no --with-java=yes
--with-python=no --with-ruby=no --with-erlang=no --with-go=no --with-nodejs=no
--with-php=no && \
-make install && \
-cd ${base_dir}
+apt-get install -y --no-install-recommends vim wget curl git automake
bison flex g++ libboost-all-dev libevent-dev libssl-dev libtool make pkg-config
&& \
+rm -rf /var/lib/apt/lists/*
+
+# install maven
+RUN mkdir -p /usr/share/maven /usr/share/maven/ref \
+ && wget
https://www-us.apache.org/dist/maven/maven-3/3.6.3/binaries/apache-maven-3.6.3-bin.tar.gz
-P /tmp \
+ && tar -xzf /tmp/apache-maven-*.tar.gz -C /usr/share/maven
--strip-components=1 \
+ && rm -f /tmp/apache-maven-*.tar.gz \
+ && ln -s /usr/share/maven/bin/mvn /usr/bin/mvn
+ENV MAVEN_HOME /usr/share/maven
+ENV MAVEN_CONFIG /opt/.m2
+
+# install thrift
+RUN wget http://archive.apache.org/dist/thrift/0.12.0/thrift-0.12.0.tar.gz -O
/tmp/thrift-0.12.0.tar.gz && \
+ tar xfz /tmp/thrift-0.12.0.tar.gz --directory /tmp && \
+ base_dir=`pwd` && \
+ cd /tmp/thrift-0.12.0 && \
+ ./configure --with-cpp=no --with-c_glib=no --with-java=yes
--with-python=no --with-ruby=no --with-erlang=no --with-go=no --with-nodejs=no
--with-php=no && \
+ make install
RUN git clone ${PINOT_GIT_URL} ${PINOT_BUILD_DIR} && \
cd ${PINOT_BUILD_DIR} && \
@@ -49,7 +59,7 @@ RUN git clone ${PINOT_GIT_URL} ${PINOT_BUILD_DIR} && \
cp -r pinot-distribution/target/apache-pinot-*-bin/apache-pinot-*-bin/*
${PINOT_HOME}/. && \
chmod +x ${PINOT_HOME}/bin/*.sh && \
mvn dependency:purge-local-repository -DactTransitively=false
-DreResolve=false --fail-at-end && \
-rm -rf ${PINOT_BUILD_DIR}
+rm -rf ${PINOT_BUILD_DIR} ${MAVEN_HOME} ${MAVEN_CONFIG}
VOLUME ["${PINOT_HOME}/configs", "${PINOT_HOME}/data"]
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch upgrade_docker_build_maven_to_3.6 created (now d5fcaad)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch upgrade_docker_build_maven_to_3.6 in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. at d5fcaad Update dockerfile for pinot image and pinot-presto image This branch includes the following new commits: new d5fcaad Update dockerfile for pinot image and pinot-presto image The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 opened a new pull request #4997: Update dockerfile for pinot image and pinot-presto image
fx19880617 opened a new pull request #4997: Update dockerfile for pinot image and pinot-presto image URL: https://github.com/apache/incubator-pinot/pull/4997 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io commented on issue #4997: Update dockerfile for pinot image and pinot-presto image
codecov-io commented on issue #4997: Update dockerfile for pinot image and pinot-presto image URL: https://github.com/apache/incubator-pinot/pull/4997#issuecomment-575875680 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4997?src=pr&el=h1) Report > Merging [#4997](https://codecov.io/gh/apache/incubator-pinot/pull/4997?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/f990656ee442973d311157bfca993db6d8218874?src=pr&el=desc) will **decrease** coverage by `0.07%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4997?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#4997 +/- ## - Coverage 57.26% 57.18% -0.08% Complexity 12 12 Files 1177 1177 Lines 6245562455 Branches 9174 9174 - Hits 3576535716 -49 - Misses2402024066 +46 - Partials 2670 2673 +3 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4997?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...er/validation/BrokerResourceValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4997/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL0Jyb2tlclJlc291cmNlVmFsaWRhdGlvbk1hbmFnZXIuamF2YQ==) | `25% <0%> (-56.25%)` | `0% <0%> (ø)` | | | [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4997/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `50% <0%> (-25%)` | `0% <0%> (ø)` | | | [.../impl/dictionary/FloatOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4997/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPbkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh) | `47.56% <0%> (-13.42%)` | `0% <0%> (ø)` | | | [...impl/dictionary/StringOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4997/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvU3RyaW5nT25IZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `77.58% <0%> (-10.35%)` | `0% <0%> (ø)` | | | [...e/operator/dociditerators/SortedDocIdIterator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4997/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9Tb3J0ZWREb2NJZEl0ZXJhdG9yLmphdmE=) | `47.22% <0%> (-8.34%)` | `0% <0%> (ø)` | | | [.../impl/dictionary/LongOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4997/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09mZkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh) | `53.76% <0%> (-6.46%)` | `0% <0%> (ø)` | | | [...impl/dictionary/DoubleOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4997/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT25IZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `39.02% <0%> (-6.1%)` | `0% <0%> (ø)` | | | [...troller/helix/core/retention/RetentionManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4997/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JldGVudGlvbi9SZXRlbnRpb25NYW5hZ2VyLmphdmE=) | `75% <0%> (-4.17%)` | `0% <0%> (ø)` | | | [...org/apache/pinot/client/DynamicBrokerSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/4997/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0R5bmFtaWNCcm9rZXJTZWxlY3Rvci5qYXZh) | `69.69% <0%> (-3.04%)` | `0% <0%> (ø)` | | | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/4997/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `47.61% <0%> (-2.39%)` | `0% <0%> (ø)` | | | ... and [16 more](https://codecov.io/gh/apache/incubator-pinot/pull/4997/diff?src=pr&el=tree-more) | | -- [Continue to review full report at
