[GitHub] [incubator-pinot] kishoreg commented on issue #4869: Adding pinot-spi module and moving record reader interface
kishoreg commented on issue #4869: Adding pinot-spi module and moving record reader interface URL: https://github.com/apache/incubator-pinot/pull/4869#issuecomment-559378103 Jackie had also approved this earlier and we were waiting for build to pass 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 merged pull request #4869: Adding pinot-spi module and moving record reader interface
kishoreg merged pull request #4869: Adding pinot-spi module and moving record reader interface URL: https://github.com/apache/incubator-pinot/pull/4869 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 issue #4869: Adding pinot-spi module and moving record reader interface
kishoreg commented on issue #4869: Adding pinot-spi module and moving record reader interface URL: https://github.com/apache/incubator-pinot/pull/4869#issuecomment-559377587 Merging so that others can push changes. 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 #4869: Adding pinot-spi module and moving record reader interface
codecov-io commented on issue #4869: Adding pinot-spi module and moving record reader interface URL: https://github.com/apache/incubator-pinot/pull/4869#issuecomment-559318125 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4869?src=pr=h1) Report > Merging [#4869](https://codecov.io/gh/apache/incubator-pinot/pull/4869?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1ba62059e8d6b8386e94d8d71a44b52db6fbf329?src=pr=desc) will **increase** coverage by `0.01%`. > The diff coverage is `27.58%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4869?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4869 +/- ## + Coverage 57.74% 57.75% +0.01% Complexity44 Files 1200 1201 +1 Lines 6465864659 +1 Branches 9390 9389 -1 + Hits 3733437342 +8 + Misses2455324543 -10 - Partials 2771 2774 +3 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4869?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [.../data/aggregator/PercentileEstValueAggregator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4869/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL2FnZ3JlZ2F0b3IvUGVyY2VudGlsZUVzdFZhbHVlQWdncmVnYXRvci5qYXZh) | `100% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...inot/core/data/readers/GenericRowRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4869/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3JlYWRlcnMvR2VuZXJpY1Jvd1JlY29yZFJlYWRlci5qYXZh) | `93.33% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...org/apache/pinot/core/minion/SegmentConverter.java](https://codecov.io/gh/apache/incubator-pinot/pull/4869/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9taW5pb24vU2VnbWVudENvbnZlcnRlci5qYXZh) | `71.95% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...e/realtime/impl/kafka/KafkaAvroMessageDecoder.java](https://codecov.io/gh/apache/incubator-pinot/pull/4869/diff?src=pr=tree#diff-cGlub3QtY29ubmVjdG9ycy9waW5vdC1jb25uZWN0b3Ita2Fma2EtYmFzZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2thZmthL0thZmthQXZyb01lc3NhZ2VEZWNvZGVyLmphdmE=) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...re/startree/v2/store/StarTreeMetricDataSource.java](https://codecov.io/gh/apache/incubator-pinot/pull/4869/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS92Mi9zdG9yZS9TdGFyVHJlZU1ldHJpY0RhdGFTb3VyY2UuamF2YQ==) | `54.16% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...ot/core/startree/v2/store/StarTreeLoaderUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4869/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS92Mi9zdG9yZS9TdGFyVHJlZUxvYWRlclV0aWxzLmphdmE=) | `93.65% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...t/common/response/broker/BrokerResponseNative.java](https://codecov.io/gh/apache/incubator-pinot/pull/4869/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzcG9uc2UvYnJva2VyL0Jyb2tlclJlc3BvbnNlTmF0aXZlLmphdmE=) | `91.57% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...not/core/plan/maker/BrokerRequestPreProcessor.java](https://codecov.io/gh/apache/incubator-pinot/pull/4869/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL21ha2VyL0Jyb2tlclJlcXVlc3RQcmVQcm9jZXNzb3IuamF2YQ==) | `75% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...va/org/apache/pinot/common/config/TableConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/4869/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL1RhYmxlQ29uZmlnLmphdmE=) | `83.83% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...ot/common/protocols/SegmentCompletionProtocol.java](https://codecov.io/gh/apache/incubator-pinot/pull/4869/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcHJvdG9jb2xzL1NlZ21lbnRDb21wbGV0aW9uUHJvdG9jb2wuamF2YQ==) | `94.05% <ø> (-0.55%)` | `0 <0> (ø)` | | | ... and [258 more](https://codecov.io/gh/apache/incubator-pinot/pull/4869/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4869?src=pr=continue). > **Legend** - [Click here to learn
[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4800: Pinot Ingestion Jobs Refactor with Pinot Spark Support
codecov-io edited a comment on issue #4800: Pinot Ingestion Jobs Refactor with Pinot Spark Support URL: https://github.com/apache/incubator-pinot/pull/4800#issuecomment-551028172 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4800?src=pr=h1) Report > Merging [#4800](https://codecov.io/gh/apache/incubator-pinot/pull/4800?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/fdfe8a2bde481aa3233a7675fbf4040a330d4b63?src=pr=desc) will **decrease** coverage by `0.83%`. > The diff coverage is `16.25%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4800?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4800 +/- ## - Coverage 57.81% 56.97% -0.84% Complexity44 Files 1200 1204 +4 Lines 6465964999 +340 Branches 9390 9470 +80 - Hits 3738537036 -349 - Misses2449425235 +741 + Partials 2780 2728 -52 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4800?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [.../org/apache/pinot/hadoop/io/PinotOutputFormat.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3AvaW8vUGlub3RPdXRwdXRGb3JtYXQuamF2YQ==) | `56.47% <ø> (ø)` | `0 <0> (?)` | | | [...che/pinot/ingestion/common/JobConfigConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaW5nZXN0aW9uLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvaW5nZXN0aW9uL2NvbW1vbi9Kb2JDb25maWdDb25zdGFudHMuamF2YQ==) | `0% <ø> (ø)` | `0 <0> (?)` | | | [.../apache/pinot/hadoop/io/JsonPinotOutputFormat.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3AvaW8vSnNvblBpbm90T3V0cHV0Rm9ybWF0LmphdmE=) | `55.17% <ø> (ø)` | `0 <0> (?)` | | | [...n/java/org/apache/pinot/hadoop/io/PinotRecord.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3AvaW8vUGlub3RSZWNvcmQuamF2YQ==) | `42.1% <ø> (ø)` | `0 <0> (?)` | | | [...org/apache/pinot/ingestion/utils/PushLocation.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaW5nZXN0aW9uLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvaW5nZXN0aW9uL3V0aWxzL1B1c2hMb2NhdGlvbi5qYXZh) | `72.72% <ø> (ø)` | `0 <0> (?)` | | | [...che/pinot/hadoop/io/CombineAvroKeyInputFormat.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3AvaW8vQ29tYmluZUF2cm9LZXlJbnB1dEZvcm1hdC5qYXZh) | `20% <ø> (ø)` | `0 <0> (?)` | | | [.../org/apache/pinot/hadoop/io/PinotRecordWriter.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3AvaW8vUGlub3RSZWNvcmRXcml0ZXIuamF2YQ==) | `88.88% <ø> (ø)` | `0 <0> (?)` | | | [...inot/hadoop/job/mappers/SegmentCreationMapper.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3Avam9iL21hcHBlcnMvU2VnbWVudENyZWF0aW9uTWFwcGVyLmphdmE=) | `1.09% <ø> (ø)` | `0 <0> (?)` | | | [...he/pinot/ingestion/utils/JobPreparationHelper.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaW5nZXN0aW9uLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvaW5nZXN0aW9uL3V0aWxzL0pvYlByZXBhcmF0aW9uSGVscGVyLmphdmE=) | `0% <ø> (ø)` | `0 <0> (?)` | | | [...doop/job/reducers/SegmentPreprocessingReducer.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3Avam9iL3JlZHVjZXJzL1NlZ21lbnRQcmVwcm9jZXNzaW5nUmVkdWNlci5qYXZh) | `0% <ø> (ø)` | `0 <0> (?)` | | | ... and [76 more](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree-more) | | -- [Continue to review full report at
[GitHub] [incubator-pinot] fx19880617 closed pull request #4796: [In Progress] Pinot Spark
fx19880617 closed pull request #4796: [In Progress] Pinot Spark URL: https://github.com/apache/incubator-pinot/pull/4796 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 #4866: Removing segment generation config from RecordReader interface
codecov-io edited a comment on issue #4866: Removing segment generation config from RecordReader interface URL: https://github.com/apache/incubator-pinot/pull/4866#issuecomment-558984119 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4866?src=pr=h1) Report > Merging [#4866](https://codecov.io/gh/apache/incubator-pinot/pull/4866?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/fdfe8a2bde481aa3233a7675fbf4040a330d4b63?src=pr=desc) will **decrease** coverage by `0.03%`. > The diff coverage is `41.17%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4866?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4866 +/- ## - Coverage 57.81% 57.77% -0.04% Complexity44 Files 1200 1200 Lines 6465964658 -1 Branches 9390 9390 - Hits 3738537359 -26 - Misses2449424514 +20 - Partials 2780 2785 +5 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4866?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...inot/core/data/readers/GenericRowRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3JlYWRlcnMvR2VuZXJpY1Jvd1JlY29yZFJlYWRlci5qYXZh) | `93.33% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...ache/pinot/core/data/readers/JSONRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3JlYWRlcnMvSlNPTlJlY29yZFJlYWRlci5qYXZh) | `86.66% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [.../pinot/core/minion/segment/MapperRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9taW5pb24vc2VnbWVudC9NYXBwZXJSZWNvcmRSZWFkZXIuamF2YQ==) | `81.08% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...pinot/core/minion/segment/ReducerRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9taW5pb24vc2VnbWVudC9SZWR1Y2VyUmVjb3JkUmVhZGVyLmphdmE=) | `90.19% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...ache/pinot/core/data/readers/AvroRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3JlYWRlcnMvQXZyb1JlY29yZFJlYWRlci5qYXZh) | `86.2% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...data/readers/MultiplePinotSegmentRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3JlYWRlcnMvTXVsdGlwbGVQaW5vdFNlZ21lbnRSZWNvcmRSZWFkZXIuamF2YQ==) | `72.22% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...pache/pinot/core/data/readers/CSVRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3JlYWRlcnMvQ1NWUmVjb3JkUmVhZGVyLmphdmE=) | `69.81% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...ealtime/converter/RealtimeSegmentRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9jb252ZXJ0ZXIvUmVhbHRpbWVTZWdtZW50UmVjb3JkUmVhZGVyLmphdmE=) | `58.33% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...he/pinot/core/data/readers/ThriftRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3JlYWRlcnMvVGhyaWZ0UmVjb3JkUmVhZGVyLmphdmE=) | `82% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...e/pinot/core/data/readers/RecordReaderFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3JlYWRlcnMvUmVjb3JkUmVhZGVyRmFjdG9yeS5qYXZh) | `34.78% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: | | ... and [38 more](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4866?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ =
[incubator-pinot] branch create-pinot-spi updated (30b4778 -> 504b26b)
This is an automated email from the ASF dual-hosted git repository. jackie pushed a change to branch create-pinot-spi in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard 30b4778 Fixing compilation issues after rebase discard c4db6a6 Adding pinot-spi module and moving recordreader interface to it discard aae3cc3 Misc clean up for PinotHelixResourceManager (#4865) discard 671dc48 [TE] Check if snapshot exists before attaching file (#4867) discard 2854b4e Enhance TagNameUtils to handle the TagOverrideConfig (#4859) discard e8da49f Fix the usage of removed guava API (#4864) discard 7bc5c63 Introduce the base JSON config class and clean up the config classes (#4858) omit 24f8e62 Modify the method signature omit 5942774 Removing segment generation config from RecordReader interface add 9881764 Introduce the base JSON config class and clean up the config classes (#4858) add 8ba44af Fix the usage of removed guava API (#4864) add 05c89fc Enhance TagNameUtils to handle the TagOverrideConfig (#4859) add 03ae23c [TE] Check if snapshot exists before attaching file (#4867) add fdfe8a2 Misc clean up for PinotHelixResourceManager (#4865) add e36c980 Removing segment generation config from RecordReader interface add d1009d4 Modify the method signature add 4bb1dc6 Adding pinot-spi module and moving recordreader interface to it add 504b26b Fixing compilation issues after rebase 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 (30b4778) \ N -- N -- N refs/heads/create-pinot-spi (504b26b) 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: - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch create-pinot-spi updated (504b26b -> 4b9c050)
This is an automated email from the ASF dual-hosted git repository. jackie pushed a change to branch create-pinot-spi in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard 504b26b Fixing compilation issues after rebase discard 4bb1dc6 Adding pinot-spi module and moving recordreader interface to it discard d1009d4 Modify the method signature discard e36c980 Removing segment generation config from RecordReader interface add 1ba6205 Removing segment generation config from RecordReader interface (#4866) add 2502359 Adding pinot-spi module and moving recordreader interface to it add 4b9c050 Fixing compilation issues after rebase 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 (504b26b) \ N -- N -- N refs/heads/create-pinot-spi (4b9c050) 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: - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated (fdfe8a2 -> 1ba6205)
This is an automated email from the ASF dual-hosted git repository. jackie pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. from fdfe8a2 Misc clean up for PinotHelixResourceManager (#4865) add 1ba6205 Removing segment generation config from RecordReader interface (#4866) No new revisions were added by this update. Summary of changes: .../pinot/core/data/readers/AvroRecordReader.java | 5 ++--- .../pinot/core/data/readers/CSVRecordReader.java | 11 + .../core/data/readers/GenericRowRecordReader.java | 6 ++--- .../pinot/core/data/readers/JSONRecordReader.java | 4 ++-- .../readers/MultiplePinotSegmentRecordReader.java | 10 - .../data/readers/PinotSegmentRecordReader.java | 10 - .../pinot/core/data/readers/RecordReader.java | 12 +++--- .../core/data/readers/RecordReaderFactory.java | 2 +- .../core/data/readers/ThriftRecordReader.java | 11 + .../generator/SegmentGeneratorConfig.java | 2 +- .../pinot/core/minion/BackfillDateTimeColumn.java | 11 + .../apache/pinot/core/minion/SegmentPurger.java| 12 +- .../core/minion/segment/MapperRecordReader.java| 6 ++--- .../core/minion/segment/ReducerRecordReader.java | 6 ++--- .../converter/RealtimeSegmentRecordReader.java | 13 ++- .../pinot/orc/data/readers/ORCRecordReader.java| 17 ++ .../orc/data/readers/ORCRecordReaderTest.java | 26 +++--- .../parquet/data/readers/ParquetRecordReader.java | 10 + .../data/readers/ParquetRecordReaderTest.java | 7 +- .../tools/admin/command/CreateSegmentCommand.java | 8 --- 20 files changed, 87 insertions(+), 102 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #4866: Removing segment generation config from RecordReader interface
Jackie-Jiang merged pull request #4866: Removing segment generation config from RecordReader interface URL: https://github.com/apache/incubator-pinot/pull/4866 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 create-pinot-spi updated (17ab98d -> 30b4778)
This is an automated email from the ASF dual-hosted git repository. jackie pushed a change to branch create-pinot-spi in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. omit 17ab98d Fixing compilation issues after rebase omit 4960f2b Changing the signature of RecordReader.init based on feedback omit 2dd6442 Adding pinot-spi module and moving recordreader interface to it omit 7fbb67e Removing segment generation config from RecordReader interface omit fdfe8a2 Misc clean up for PinotHelixResourceManager (#4865) omit 03ae23c [TE] Check if snapshot exists before attaching file (#4867) omit 05c89fc Enhance TagNameUtils to handle the TagOverrideConfig (#4859) omit 8ba44af Fix the usage of removed guava API (#4864) omit 9881764 Introduce the base JSON config class and clean up the config classes (#4858) add 5942774 Removing segment generation config from RecordReader interface add 24f8e62 Modify the method signature add 7bc5c63 Introduce the base JSON config class and clean up the config classes (#4858) add e8da49f Fix the usage of removed guava API (#4864) add 2854b4e Enhance TagNameUtils to handle the TagOverrideConfig (#4859) add 671dc48 [TE] Check if snapshot exists before attaching file (#4867) add aae3cc3 Misc clean up for PinotHelixResourceManager (#4865) add c4db6a6 Adding pinot-spi module and moving recordreader interface to it add 30b4778 Fixing compilation issues after rebase 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 (17ab98d) \ N -- N -- N refs/heads/create-pinot-spi (30b4778) 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/core/data/readers/AvroRecordReader.java | 5 ++-- .../pinot/core/data/readers/CSVRecordReader.java | 11 - .../core/data/readers/GenericRowRecordReader.java | 5 ++-- .../pinot/core/data/readers/JSONRecordReader.java | 7 +++--- .../readers/MultiplePinotSegmentRecordReader.java | 10 .../data/readers/PinotSegmentRecordReader.java | 10 .../core/data/readers/ThriftRecordReader.java | 11 - .../pinot/core/minion/BackfillDateTimeColumn.java | 27 ++ .../apache/pinot/core/minion/SegmentPurger.java| 13 --- .../core/minion/segment/MapperRecordReader.java| 7 +++--- .../core/minion/segment/ReducerRecordReader.java | 7 +++--- .../converter/RealtimeSegmentRecordReader.java | 13 +-- .../pinot/orc/data/readers/ORCRecordReader.java| 14 --- .../orc/data/readers/ORCRecordReaderTest.java | 15 +--- .../parquet/data/readers/ParquetRecordReader.java | 3 ++- .../data/readers/ParquetRecordReaderTest.java | 1 - .../pinot/spi/data/readers/RecordReader.java | 11 - .../tools/admin/command/CreateSegmentCommand.java | 3 ++- 18 files changed, 74 insertions(+), 99 deletions(-) - 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 #4870: Add getResultColumnName API to AggregationFunction to fetch sql compliant result name
codecov-io edited a comment on issue #4870: Add getResultColumnName API to AggregationFunction to fetch sql compliant result name URL: https://github.com/apache/incubator-pinot/pull/4870#issuecomment-559263340 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4870?src=pr=h1) Report > :exclamation: No coverage uploaded for pull request base (`master@03ae23c`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit). > The diff coverage is `56.75%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4870?src=pr=tree) ```diff @@ Coverage Diff@@ ## master #4870 +/- ## Coverage ? 57.7% Complexity? 4 Files ?1200 Lines ? 64684 Branches ?9390 Hits ? 37325 Misses? 24587 Partials ?2772 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4870?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...aggregation/function/AggregationFunctionUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9BZ2dyZWdhdGlvbkZ1bmN0aW9uVXRpbHMuamF2YQ==) | `95.55% <ø> (ø)` | `0 <0> (?)` | | | [...y/aggregation/function/MaxAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9NYXhBZ2dyZWdhdGlvbkZ1bmN0aW9uLmphdmE=) | `94.87% <0%> (ø)` | `0 <0> (?)` | | | [...nction/PercentileTDigestMVAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlVERpZ2VzdE1WQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `66.66% <0%> (ø)` | `0 <0> (?)` | | | [...nction/DistinctCountRawHLLAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9EaXN0aW5jdENvdW50UmF3SExMQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `88% <0%> (ø)` | `0 <0> (?)` | | | [.../function/DistinctCountHLLAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9EaXN0aW5jdENvdW50SExMQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `42.85% <0%> (ø)` | `0 <0> (?)` | | | [...ation/function/MinMaxRangeAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9NaW5NYXhSYW5nZUFnZ3JlZ2F0aW9uRnVuY3Rpb24uamF2YQ==) | `86.25% <0%> (ø)` | `0 <0> (?)` | | | [...function/PercentileTDigestAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlVERpZ2VzdEFnZ3JlZ2F0aW9uRnVuY3Rpb24uamF2YQ==) | `73.8% <0%> (ø)` | `0 <0> (?)` | | | [...regation/function/DistinctAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9EaXN0aW5jdEFnZ3JlZ2F0aW9uRnVuY3Rpb24uamF2YQ==) | `75.8% <0%> (ø)` | `0 <0> (?)` | | | [...aggregation/function/MaxMVAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9NYXhNVkFnZ3JlZ2F0aW9uRnVuY3Rpb24uamF2YQ==) | `96.96% <0%> (ø)` | `0 <0> (?)` | | | [...ion/function/PercentileEstAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlRXN0QWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `75.29% <0%> (ø)` | `0 <0> (?)` | | | ... and [20 more](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4870?src=pr=continue).
[incubator-pinot] branch create-pinot-spi updated (4960f2b -> 17ab98d)
This is an automated email from the ASF dual-hosted git repository. kishoreg pushed a change to branch create-pinot-spi in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. from 4960f2b Changing the signature of RecordReader.init based on feedback add 17ab98d Fixing compilation issues after rebase No new revisions were added by this update. Summary of changes: .../main/java/org/apache/pinot/common/config/BaseJsonConfig.java | 2 +- .../org/apache/pinot/common/config/SegmentPartitionConfig.java| 2 +- .../src/main/java/org/apache/pinot/common/config/TableConfig.java | 2 +- .../main/java/org/apache/pinot/common/data/StarTreeIndexSpec.java | 2 +- .../src/main/java/org/apache/pinot/startree/hll/HllConfig.java| 2 +- .../pinot/controller/helix/core/minion/ClusterInfoProvider.java | 2 +- .../java/org/apache/pinot/controller/helix/ControllerTest.java| 8 .../org/apache/pinot/core/data/readers/RecordReaderFactory.java | 2 +- .../java/org/apache/pinot/orc/data/readers/ORCRecordReader.java | 2 +- .../org/apache/pinot/orc/data/readers/ORCRecordReaderTest.java| 4 ++-- .../apache/pinot/parquet/data/readers/ParquetRecordReader.java| 2 +- .../pinot/parquet/data/readers/ParquetRecordReaderTest.java | 2 +- .../apache/pinot/tools/admin/command/CreateSegmentCommand.java| 4 ++-- 13 files changed, 18 insertions(+), 18 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch pinot-ingestion-refactor updated (5bc3ebb -> 3ffb018)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch pinot-ingestion-refactor in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard 5bc3ebb Update Readme to refer to docs discard 3601d26 Adding docs for pinot spark usage discard 6fe1c77 Add linking discard bd69212 upgrade version discard 99e7bc9 Adding support for retry and back-off discard 8a6d36b update with new API discard 0e031f4 Adding config to control spark job push parallism discard bb900bf Fixing parent directory creation issue discard 37c1bfd don't use default conf discard 74cc45c fixing class: com.databricks.backend.daemon.data.client.DBFS not serializable issue discard 2362a07 Ensure segment push job is Serializable discard 02f1f6a Make spark job an option to parallel push segments discard 8a0a506 Address comments discard 10823e1 Move segment tars recursively and override existed files discard 3fe3ee4 Initial refactor discard e6639c4 Make temp directory with uuid appended discard 3866151 Make PushLocation Serializable discard baf3e38 Use SparkContext.getOrCreate() to use shared SparkContext if possible. discard 69f68b0 Initial commit for pinot-spark add 9881764 Introduce the base JSON config class and clean up the config classes (#4858) add 8ba44af Fix the usage of removed guava API (#4864) add 05c89fc Enhance TagNameUtils to handle the TagOverrideConfig (#4859) add 03ae23c [TE] Check if snapshot exists before attaching file (#4867) add fdfe8a2 Misc clean up for PinotHelixResourceManager (#4865) add 8c72408 Initial commit for pinot-spark add 2dadcd6 Use SparkContext.getOrCreate() to use shared SparkContext if possible. add 1715be3 Make PushLocation Serializable add 511f1a9 Make temp directory with uuid appended add 5281119 Initial refactor add 0152259 Move segment tars recursively and override existed files add 78d0b5b Address comments add fcbbd47 Make spark job an option to parallel push segments add 32f1fa3 Ensure segment push job is Serializable add 89c4522 fixing class: com.databricks.backend.daemon.data.client.DBFS not serializable issue add 24d9549 don't use default conf add 171532c Fixing parent directory creation issue add 4fe4904 Adding config to control spark job push parallism add 45abb1b update with new API add 146d54a Adding support for retry and back-off add 12f1e2c upgrade version add e325428 Add linking add c975433 Adding docs for pinot spark usage add 3ffb018 Update Readme to refer to docs 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 (5bc3ebb) \ N -- N -- N refs/heads/pinot-ingestion-refactor (3ffb018) 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: .../broker/broker/helix/HelixBrokerStarter.java| 21 +- .../routing/builder/BaseRoutingTableBuilder.java | 5 +- ...elixExternalViewBasedQueryQuotaManagerTest.java | 31 +-- .../BalancedRandomRoutingTableBuilderTest.java | 8 +- ...rtitionAwareOfflineRoutingTableBuilderTest.java | 7 +- ...titionAwareRealtimeRoutingTableBuilderTest.java | 3 +- .../builder/RoutingTableBuilderTestUtil.java | 44 .../common/assignment/InstancePartitionsUtils.java | 11 +- .../Segment.java => config/BaseJsonConfig.java}| 39 +-- .../pinot/common/config/ColumnPartitionConfig.java | 40 +--- .../pinot/common/config/CompletionConfig.java | 40 +--- .../apache/pinot/common/config/IndexingConfig.java | 94 +--- .../org/apache/pinot/common/config/Instance.java | 22 +- .../pinot/common/config/OfflineTagConfig.java | 39 --- .../apache/pinot/common/config/QuotaConfig.java| 49 +--- .../pinot/common/config/RealtimeTagConfig.java | 64 - .../common/config/ReplicaGroupStrategyConfig.java | 80 ++- .../apache/pinot/common/config/RoutingConfig.java | 53 ++--- .../common/config/SegmentPartitionConfig.java | 69 +- .../SegmentsValidationAndRetentionConfig.java | 157 +++-- .../pinot/common/config/StarTreeIndexConfig.java | 48 ++-- .../apache/pinot/common/config/TableConfig.java| 60 + .../pinot/common/config/TableCustomConfig.java | 76 +- .../pinot/common/config/TableNameBuilder.java | 22 +-
[GitHub] [incubator-pinot] kishoreg opened a new pull request #4866: Removing segment generation config from RecordReader interface
kishoreg opened a new pull request #4866: Removing segment generation config from RecordReader interface URL: https://github.com/apache/incubator-pinot/pull/4866 Related to #4731 refactoring the existing code to prepare for moving RecordReader to pinot-spi module. As of today, RecordReader takes in SegmentGenerationConfig which makes it very hard to extract RecordReader to pinot-spi. current interface `void init(SegmentGenerationConfig config)` new interface `void init(String inputPath, Schema schema, ReaderConfig readerConfig)` Next step: - Move record reader interface to pinot-spi module - Move Schema and related classes - Instantiate record readers dynamically during segment generation (this is optional and might do it later depending on how easy/hard). We don't need it until we last steps 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 #4870: Add getResultColumnName API to AggregationFunction to fetch sql compliant result name
npawar commented on a change in pull request #4870: Add getResultColumnName API
to AggregationFunction to fetch sql compliant result name
URL: https://github.com/apache/incubator-pinot/pull/4870#discussion_r351530958
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/table/TableResizer.java
##
@@ -57,15 +57,18 @@
int numAggregations = aggregationInfos.size();
int numKeyColumns = numColumns - numAggregations;
+int aggIdx = 0;
Map columnIndexMap = new HashMap<>();
+Map aggregationColumnToFunction = new
HashMap<>();
for (int i = 0; i < numColumns; i++) {
- columnIndexMap.put(dataSchema.getColumnName(i), i);
-}
-
-Map aggregationColumnToInfo = new HashMap<>();
-for (AggregationInfo aggregationInfo : aggregationInfos) {
- String aggregationColumn =
AggregationFunctionUtils.getAggregationColumnName(aggregationInfo);
- aggregationColumnToInfo.put(aggregationColumn, aggregationInfo);
+ String columnName = dataSchema.getColumnName(i).toLowerCase();
Review comment:
Changed this. I ended up making function name lower case in the
AggregationFunction::getResultColumnName, because TransformExpressionTree
converts function names to lower case when building the broker request. Hence
order by only has all lower case function names
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 #4870: Add getResultColumnName API to AggregationFunction to fetch sql compliant result name
npawar commented on issue #4870: Add getResultColumnName API to AggregationFunction to fetch sql compliant result name URL: https://github.com/apache/incubator-pinot/pull/4870#issuecomment-559280223 > This is good to have for now since we don't support aliasing. But this should be typically solved at a very high level using a mapping table. Let's make sure that this will not make it difficult to implement aliasing later. I feel this is required. We have no other way to fetch the name like `avg(foo)`. Order by cannot understand `avg_foo`, as the TransformExpressionTree converts it to `avg(foo)`. I don't think this should affect ability to handle aliases later 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 #4870: Add getResultColumnName API to AggregationFunction to fetch sql compliant result name
npawar commented on a change in pull request #4870: Add getResultColumnName API
to AggregationFunction to fetch sql compliant result name
URL: https://github.com/apache/incubator-pinot/pull/4870#discussion_r351530639
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/table/TableResizer.java
##
@@ -57,15 +57,18 @@
int numAggregations = aggregationInfos.size();
int numKeyColumns = numColumns - numAggregations;
+int aggIdx = 0;
Map columnIndexMap = new HashMap<>();
+Map aggregationColumnToFunction = new
HashMap<>();
for (int i = 0; i < numColumns; i++) {
- columnIndexMap.put(dataSchema.getColumnName(i), i);
-}
-
-Map aggregationColumnToInfo = new HashMap<>();
-for (AggregationInfo aggregationInfo : aggregationInfos) {
- String aggregationColumn =
AggregationFunctionUtils.getAggregationColumnName(aggregationInfo);
- aggregationColumnToInfo.put(aggregationColumn, aggregationInfo);
+ String columnName = dataSchema.getColumnName(i).toLowerCase();
+ columnIndexMap.put(columnName, i);
+ if (i >= numKeyColumns) {
+AggregationFunction aggregationFunction =
Review comment:
There's a comment at the beginning of this method (few lines above this code
change).
Fixed the other suggestion
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 closed pull request #4866: Removing segment generation config from RecordReader interface
kishoreg closed pull request #4866: Removing segment generation config from RecordReader interface URL: https://github.com/apache/incubator-pinot/pull/4866 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 issue #4866: Removing segment generation config from RecordReader interface
kishoreg commented on issue #4866: Removing segment generation config from RecordReader interface URL: https://github.com/apache/incubator-pinot/pull/4866#issuecomment-559277431 #4869 captures the changes in this PR and incorporates the feedback. Closing this PR 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 create-pinot-spi updated (2dd6442 -> 4960f2b)
This is an automated email from the ASF dual-hosted git repository. kishoreg pushed a change to branch create-pinot-spi in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. from 2dd6442 Adding pinot-spi module and moving recordreader interface to it add 4960f2b Changing the signature of RecordReader.init based on feedback No new revisions were added by this update. Summary of changes: .../java/org/apache/pinot/core/data/readers/AvroRecordReader.java| 2 +- .../java/org/apache/pinot/core/data/readers/CSVRecordReader.java | 2 +- .../org/apache/pinot/core/data/readers/GenericRowRecordReader.java | 3 ++- .../java/org/apache/pinot/core/data/readers/JSONRecordReader.java| 2 +- .../pinot/core/data/readers/MultiplePinotSegmentRecordReader.java| 2 +- .../org/apache/pinot/core/data/readers/PinotSegmentRecordReader.java | 2 +- .../java/org/apache/pinot/core/data/readers/ThriftRecordReader.java | 2 +- .../java/org/apache/pinot/core/minion/BackfillDateTimeColumn.java| 2 +- .../src/main/java/org/apache/pinot/core/minion/SegmentPurger.java| 2 +- .../org/apache/pinot/core/minion/segment/MapperRecordReader.java | 2 +- .../org/apache/pinot/core/minion/segment/ReducerRecordReader.java| 2 +- .../pinot/core/realtime/converter/RealtimeSegmentRecordReader.java | 3 ++- .../main/java/org/apache/pinot/orc/data/readers/ORCRecordReader.java | 5 +++-- .../org/apache/pinot/parquet/data/readers/ParquetRecordReader.java | 5 +++-- .../main/java/org/apache/pinot/spi/data/readers/RecordReader.java| 5 +++-- 15 files changed, 23 insertions(+), 18 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #4866: Removing segment generation config from RecordReader interface
Jackie-Jiang commented on a change in pull request #4866: Removing segment
generation config from RecordReader interface
URL: https://github.com/apache/incubator-pinot/pull/4866#discussion_r351524022
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/readers/RecordReader.java
##
@@ -34,10 +34,16 @@
public interface RecordReader extends Closeable {
/**
- * Initializes the record reader when needed
+ * initializing recordreader with inputpath, schema and recordreader config.
+ * The implementation can chose to ignore one or more of these parameters
and handle null gracefully
+ *
+ * @param inputPath absolute path to the file/directory
+ * @param schema Pinot Schema associated with the table
+ * @param recordReaderConfig config for the reader specific to the format.
e.g. delimiter for csv format etc
+ * @throws Exception if the arguments are invalid
*/
- void init(SegmentGeneratorConfig segmentGeneratorConfig)
- throws IOException;
+ void init(@Nullable String inputPath, @Nullable Schema schema, @Nullable
RecordReaderConfig recordReaderConfig)
Review comment:
I understand most record readers don't need schema as of now, but when you
call the `init()`, you are not supposed to pass `null` as the schema
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 #4866: Removing segment generation config from RecordReader interface
kishoreg commented on a change in pull request #4866: Removing segment
generation config from RecordReader interface
URL: https://github.com/apache/incubator-pinot/pull/4866#discussion_r351523408
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/readers/RecordReader.java
##
@@ -34,10 +34,16 @@
public interface RecordReader extends Closeable {
/**
- * Initializes the record reader when needed
+ * initializing recordreader with inputpath, schema and recordreader config.
+ * The implementation can chose to ignore one or more of these parameters
and handle null gracefully
+ *
+ * @param inputPath absolute path to the file/directory
+ * @param schema Pinot Schema associated with the table
+ * @param recordReaderConfig config for the reader specific to the format.
e.g. delimiter for csv format etc
+ * @throws Exception if the arguments are invalid
*/
- void init(SegmentGeneratorConfig segmentGeneratorConfig)
- throws IOException;
+ void init(@Nullable String inputPath, @Nullable Schema schema, @Nullable
RecordReaderConfig recordReaderConfig)
Review comment:
only for Parquet and ORC readers. There are some record readers which dont
need schema as of today.
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 refactor-record-reader updated (49acc30 -> 24f8e62)
This is an automated email from the ASF dual-hosted git repository. jackie pushed a change to branch refactor-record-reader in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard 49acc30 Modify the method signature add 24f8e62 Modify the method signature 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 (49acc30) \ N -- N -- N refs/heads/refactor-record-reader (24f8e62) 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: .../org/apache/pinot/core/minion/BackfillDateTimeColumn.java | 6 ++ .../main/java/org/apache/pinot/core/minion/SegmentPurger.java| 9 - 2 files changed, 6 insertions(+), 9 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch refactor-record-reader updated (5942774 -> 49acc30)
This is an automated email from the ASF dual-hosted git repository. jackie pushed a change to branch refactor-record-reader in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. from 5942774 Removing segment generation config from RecordReader interface add 49acc30 Modify the method signature No new revisions were added by this update. Summary of changes: .../pinot/core/data/readers/AvroRecordReader.java | 6 ++ .../pinot/core/data/readers/CSVRecordReader.java | 12 +--- .../core/data/readers/GenericRowRecordReader.java | 7 +++ .../pinot/core/data/readers/JSONRecordReader.java | 6 ++ .../readers/MultiplePinotSegmentRecordReader.java | 11 --- .../core/data/readers/PinotSegmentRecordReader.java | 11 --- .../apache/pinot/core/data/readers/RecordReader.java | 14 +++--- .../pinot/core/data/readers/RecordReaderFactory.java | 2 +- .../pinot/core/data/readers/ThriftRecordReader.java | 12 +--- .../pinot/core/minion/BackfillDateTimeColumn.java | 9 - .../org/apache/pinot/core/minion/SegmentPurger.java | 4 +--- .../pinot/core/minion/segment/MapperRecordReader.java | 6 ++ .../core/minion/segment/ReducerRecordReader.java | 6 ++ .../converter/RealtimeSegmentRecordReader.java| 13 ++--- .../pinot/orc/data/readers/ORCRecordReader.java | 16 ++-- .../pinot/orc/data/readers/ORCRecordReaderTest.java | 19 --- .../parquet/data/readers/ParquetRecordReader.java | 7 --- .../parquet/data/readers/ParquetRecordReaderTest.java | 3 +-- .../tools/admin/command/CreateSegmentCommand.java | 7 --- 19 files changed, 71 insertions(+), 100 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] akshayrai commented on a change in pull request #4837: [TE] clean up some legacy code
akshayrai commented on a change in pull request #4837: [TE] clean up some legacy code URL: https://github.com/apache/incubator-pinot/pull/4837#discussion_r351518020 ## File path: thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/integration/AnomalyApplicationEndToEndTest.java ## @@ -1,435 +0,0 @@ -/** Review comment: This test is still relying on the legacy classes like DetectionJobScheduler and AlertJobSchedulerV2. It needs to be rewritten. 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 #4870: Add getResultColumnName API to AggregationFunction to fetch sql compliant result name
codecov-io commented on issue #4870: Add getResultColumnName API to AggregationFunction to fetch sql compliant result name URL: https://github.com/apache/incubator-pinot/pull/4870#issuecomment-559263340 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4870?src=pr=h1) Report > :exclamation: No coverage uploaded for pull request base (`master@03ae23c`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit). > The diff coverage is `60%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4870?src=pr=tree) ```diff @@Coverage Diff@@ ## master#4870 +/- ## = Coverage ? 57.82% Complexity?4 = Files ? 1200 Lines ?64689 Branches ? 9389 = Hits ?37404 Misses?24503 Partials ? 2782 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4870?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...aggregation/function/AggregationFunctionUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9BZ2dyZWdhdGlvbkZ1bmN0aW9uVXRpbHMuamF2YQ==) | `95.55% <ø> (ø)` | `0 <0> (?)` | | | [...y/aggregation/function/MaxAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9NYXhBZ2dyZWdhdGlvbkZ1bmN0aW9uLmphdmE=) | `94.87% <0%> (ø)` | `0 <0> (?)` | | | [...nction/PercentileTDigestMVAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlVERpZ2VzdE1WQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `66.66% <0%> (ø)` | `0 <0> (?)` | | | [...nction/DistinctCountRawHLLAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9EaXN0aW5jdENvdW50UmF3SExMQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `88% <0%> (ø)` | `0 <0> (?)` | | | [.../function/DistinctCountHLLAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9EaXN0aW5jdENvdW50SExMQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `42.85% <0%> (ø)` | `0 <0> (?)` | | | [...ation/function/MinMaxRangeAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9NaW5NYXhSYW5nZUFnZ3JlZ2F0aW9uRnVuY3Rpb24uamF2YQ==) | `86.25% <0%> (ø)` | `0 <0> (?)` | | | [...function/PercentileTDigestAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlVERpZ2VzdEFnZ3JlZ2F0aW9uRnVuY3Rpb24uamF2YQ==) | `73.8% <0%> (ø)` | `0 <0> (?)` | | | [...regation/function/DistinctAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9EaXN0aW5jdEFnZ3JlZ2F0aW9uRnVuY3Rpb24uamF2YQ==) | `75.8% <0%> (ø)` | `0 <0> (?)` | | | [...aggregation/function/MaxMVAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9NYXhNVkFnZ3JlZ2F0aW9uRnVuY3Rpb24uamF2YQ==) | `96.96% <0%> (ø)` | `0 <0> (?)` | | | [...ion/function/PercentileEstAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlRXN0QWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `75.29% <0%> (ø)` | `0 <0> (?)` | | | ... and [20 more](https://codecov.io/gh/apache/incubator-pinot/pull/4870/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4870?src=pr=continue).
[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4800: Pinot Ingestion Jobs Refactor with Pinot Spark Support
codecov-io edited a comment on issue #4800: Pinot Ingestion Jobs Refactor with Pinot Spark Support URL: https://github.com/apache/incubator-pinot/pull/4800#issuecomment-551028172 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4800?src=pr=h1) Report > :exclamation: No coverage uploaded for pull request base (`master@03ae23c`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit). > The diff coverage is `16.25%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4800?src=pr=tree) ```diff @@Coverage Diff@@ ## master#4800 +/- ## = Coverage ? 57.22% Complexity?4 = Files ? 1204 Lines ?64719 Branches ? 9396 = Hits ?37038 Misses?24948 Partials ? 2733 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4800?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [.../org/apache/pinot/hadoop/io/PinotOutputFormat.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3AvaW8vUGlub3RPdXRwdXRGb3JtYXQuamF2YQ==) | `56.47% <ø> (ø)` | `0 <0> (?)` | | | [...che/pinot/ingestion/common/JobConfigConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaW5nZXN0aW9uLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvaW5nZXN0aW9uL2NvbW1vbi9Kb2JDb25maWdDb25zdGFudHMuamF2YQ==) | `0% <ø> (ø)` | `0 <0> (?)` | | | [.../apache/pinot/hadoop/io/JsonPinotOutputFormat.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3AvaW8vSnNvblBpbm90T3V0cHV0Rm9ybWF0LmphdmE=) | `55.17% <ø> (ø)` | `0 <0> (?)` | | | [...n/java/org/apache/pinot/hadoop/io/PinotRecord.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3AvaW8vUGlub3RSZWNvcmQuamF2YQ==) | `42.1% <ø> (ø)` | `0 <0> (?)` | | | [...org/apache/pinot/ingestion/utils/PushLocation.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaW5nZXN0aW9uLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvaW5nZXN0aW9uL3V0aWxzL1B1c2hMb2NhdGlvbi5qYXZh) | `72.72% <ø> (ø)` | `0 <0> (?)` | | | [...che/pinot/hadoop/io/CombineAvroKeyInputFormat.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3AvaW8vQ29tYmluZUF2cm9LZXlJbnB1dEZvcm1hdC5qYXZh) | `20% <ø> (ø)` | `0 <0> (?)` | | | [.../org/apache/pinot/hadoop/io/PinotRecordWriter.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3AvaW8vUGlub3RSZWNvcmRXcml0ZXIuamF2YQ==) | `88.88% <ø> (ø)` | `0 <0> (?)` | | | [...inot/hadoop/job/mappers/SegmentCreationMapper.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3Avam9iL21hcHBlcnMvU2VnbWVudENyZWF0aW9uTWFwcGVyLmphdmE=) | `1.09% <ø> (ø)` | `0 <0> (?)` | | | [...he/pinot/ingestion/utils/JobPreparationHelper.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaW5nZXN0aW9uLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvaW5nZXN0aW9uL3V0aWxzL0pvYlByZXBhcmF0aW9uSGVscGVyLmphdmE=) | `0% <ø> (ø)` | `0 <0> (?)` | | | [...doop/job/reducers/SegmentPreprocessingReducer.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3Avam9iL3JlZHVjZXJzL1NlZ21lbnRQcmVwcm9jZXNzaW5nUmVkdWNlci5qYXZh) | `0% <ø> (ø)` | `0 <0> (?)` | | | ... and [16 more](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4800?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
[GitHub] [incubator-pinot] dbadapon commented on a change in pull request #4737: Druid-Pinot Segment Converter Tool
dbadapon commented on a change in pull request #4737: Druid-Pinot Segment Converter Tool URL: https://github.com/apache/incubator-pinot/pull/4737#discussion_r351510597 ## File path: pom.xml ## @@ -330,7 +330,7 @@ org.slf4j slf4j-log4j12 - + Review comment: I'm looking at the file and I don't see anything wrong with the indentation... what is happening? 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] dbadapon commented on a change in pull request #4737: Druid-Pinot Segment Converter Tool
dbadapon commented on a change in pull request #4737: Druid-Pinot Segment Converter Tool URL: https://github.com/apache/incubator-pinot/pull/4737#discussion_r351510713 ## File path: pinot-tools/pom.xml ## @@ -123,6 +123,10 @@ + Review comment: I think the same thing is happening here as above. I'm looking at the file and there does not seem to be anything wrong with the indentation, and I'm not sure how this diff was created in the first place. 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 #4800: Pinot Ingestion Jobs Refactor with Pinot Spark Support
codecov-io edited a comment on issue #4800: Pinot Ingestion Jobs Refactor with Pinot Spark Support URL: https://github.com/apache/incubator-pinot/pull/4800#issuecomment-551028172 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4800?src=pr=h1) Report > :exclamation: No coverage uploaded for pull request base (`master@03ae23c`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit). > The diff coverage is `11.33%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4800?src=pr=tree) ```diff @@Coverage Diff@@ ## master#4800 +/- ## = Coverage ? 36.58% = Files ? 1204 Lines ?64719 Branches ? 9396 = Hits ?23677 Misses?38905 Partials ? 2137 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4800?src=pr=tree) | Coverage Δ | | |---|---|---| | [.../org/apache/pinot/hadoop/io/PinotOutputFormat.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3AvaW8vUGlub3RPdXRwdXRGb3JtYXQuamF2YQ==) | `0% <ø> (ø)` | | | [...che/pinot/ingestion/common/JobConfigConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaW5nZXN0aW9uLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvaW5nZXN0aW9uL2NvbW1vbi9Kb2JDb25maWdDb25zdGFudHMuamF2YQ==) | `0% <ø> (ø)` | | | [.../apache/pinot/hadoop/io/JsonPinotOutputFormat.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3AvaW8vSnNvblBpbm90T3V0cHV0Rm9ybWF0LmphdmE=) | `0% <ø> (ø)` | | | [...n/java/org/apache/pinot/hadoop/io/PinotRecord.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3AvaW8vUGlub3RSZWNvcmQuamF2YQ==) | `0% <ø> (ø)` | | | [...org/apache/pinot/ingestion/utils/PushLocation.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaW5nZXN0aW9uLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvaW5nZXN0aW9uL3V0aWxzL1B1c2hMb2NhdGlvbi5qYXZh) | `0% <ø> (ø)` | | | [...che/pinot/hadoop/io/CombineAvroKeyInputFormat.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3AvaW8vQ29tYmluZUF2cm9LZXlJbnB1dEZvcm1hdC5qYXZh) | `20% <ø> (ø)` | | | [.../org/apache/pinot/hadoop/io/PinotRecordWriter.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3AvaW8vUGlub3RSZWNvcmRXcml0ZXIuamF2YQ==) | `0% <ø> (ø)` | | | [...inot/hadoop/job/mappers/SegmentCreationMapper.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3Avam9iL21hcHBlcnMvU2VnbWVudENyZWF0aW9uTWFwcGVyLmphdmE=) | `0% <ø> (ø)` | | | [...he/pinot/ingestion/utils/JobPreparationHelper.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaW5nZXN0aW9uLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvaW5nZXN0aW9uL3V0aWxzL0pvYlByZXBhcmF0aW9uSGVscGVyLmphdmE=) | `0% <ø> (ø)` | | | [...doop/job/reducers/SegmentPreprocessingReducer.java](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree#diff-cGlub3QtaW5nZXN0aW9uLWpvYnMvcGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3Avam9iL3JlZHVjZXJzL1NlZ21lbnRQcmVwcm9jZXNzaW5nUmVkdWNlci5qYXZh) | `0% <ø> (ø)` | | | ... and [16 more](https://codecov.io/gh/apache/incubator-pinot/pull/4800/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4800?src=pr=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/4800?src=pr=footer). Last update
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #4866: Removing segment generation config from RecordReader interface
Jackie-Jiang commented on a change in pull request #4866: Removing segment
generation config from RecordReader interface
URL: https://github.com/apache/incubator-pinot/pull/4866#discussion_r351510391
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/readers/RecordReader.java
##
@@ -34,10 +34,16 @@
public interface RecordReader extends Closeable {
/**
- * Initializes the record reader when needed
+ * initializing recordreader with inputpath, schema and recordreader config.
+ * The implementation can chose to ignore one or more of these parameters
and handle null gracefully
+ *
+ * @param inputPath absolute path to the file/directory
+ * @param schema Pinot Schema associated with the table
+ * @param recordReaderConfig config for the reader specific to the format.
e.g. delimiter for csv format etc
+ * @throws Exception if the arguments are invalid
*/
- void init(SegmentGeneratorConfig segmentGeneratorConfig)
- throws IOException;
+ void init(@Nullable String inputPath, @Nullable Schema schema, @Nullable
RecordReaderConfig recordReaderConfig)
Review comment:
When you call `init()`, you will always provide the input path and schema
right?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 create-pinot-spi updated (c6435aa -> 2dd6442)
This is an automated email from the ASF dual-hosted git repository. kishoreg pushed a change to branch create-pinot-spi in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard c6435aa Adding pinot-spi module and moving recordreader interface to it omit 5942774 Removing segment generation config from RecordReader interface add 9881764 Introduce the base JSON config class and clean up the config classes (#4858) add 8ba44af Fix the usage of removed guava API (#4864) add 05c89fc Enhance TagNameUtils to handle the TagOverrideConfig (#4859) add 03ae23c [TE] Check if snapshot exists before attaching file (#4867) add fdfe8a2 Misc clean up for PinotHelixResourceManager (#4865) add 7fbb67e Removing segment generation config from RecordReader interface add 2dd6442 Adding pinot-spi module and moving recordreader interface to it 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 (c6435aa) \ N -- N -- N refs/heads/create-pinot-spi (2dd6442) 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: .../broker/broker/helix/HelixBrokerStarter.java| 21 +- .../routing/builder/BaseRoutingTableBuilder.java | 5 +- ...elixExternalViewBasedQueryQuotaManagerTest.java | 31 +-- .../BalancedRandomRoutingTableBuilderTest.java | 8 +- ...rtitionAwareOfflineRoutingTableBuilderTest.java | 7 +- ...titionAwareRealtimeRoutingTableBuilderTest.java | 3 +- .../builder/RoutingTableBuilderTestUtil.java | 44 .../common/assignment/InstancePartitionsUtils.java | 11 +- .../Segment.java => config/BaseJsonConfig.java}| 39 +-- .../pinot/common/config/ColumnPartitionConfig.java | 40 +--- .../pinot/common/config/CompletionConfig.java | 40 +--- .../apache/pinot/common/config/IndexingConfig.java | 94 +--- .../org/apache/pinot/common/config/Instance.java | 22 +- .../pinot/common/config/OfflineTagConfig.java | 39 --- .../apache/pinot/common/config/QuotaConfig.java| 49 +--- .../pinot/common/config/RealtimeTagConfig.java | 64 - .../common/config/ReplicaGroupStrategyConfig.java | 80 ++- .../apache/pinot/common/config/RoutingConfig.java | 53 ++--- .../common/config/SegmentPartitionConfig.java | 71 +- .../SegmentsValidationAndRetentionConfig.java | 157 +++-- .../pinot/common/config/StarTreeIndexConfig.java | 48 ++-- .../apache/pinot/common/config/TableConfig.java| 62 + .../pinot/common/config/TableCustomConfig.java | 76 +- .../pinot/common/config/TableNameBuilder.java | 22 +- .../pinot/common/config/TableTaskConfig.java | 41 +--- .../org/apache/pinot/common/config/TagConfig.java | 43 .../apache/pinot/common/config/TagNameUtils.java | 144 .../pinot/common/config/TagOverrideConfig.java | 56 + .../org/apache/pinot/common/config/Tenant.java | 111 ++--- .../apache/pinot/common/config/TenantConfig.java | 95 ++-- .../config/instance/InstanceAssignmentConfig.java | 46 ++-- .../instance/InstanceAssignmentConfigUtils.java| 22 +- .../config/instance/InstanceConstraintConfig.java | 21 +- .../InstanceReplicaGroupPartitionConfig.java | 68 ++ .../config/instance/InstanceTagPoolConfig.java | 52 ++-- .../pinot/common/data/StarTreeIndexSpec.java | 55 + .../org/apache/pinot/startree/hll/HllConfig.java | 47 +--- .../pinot/common/config/TableConfigTest.java | 44 +--- .../pinot/common/config/TagOverrideConfigTest.java | 92 ++-- .../org/apache/pinot/common/config/TenantTest.java | 9 +- .../api/resources/PinotSegmentRestletResource.java | 2 +- .../PinotSegmentUploadDownloadRestletResource.java | 4 +- .../helix/core/PinotHelixResourceManager.java | 261 ++--- .../helix/core/PinotTableIdealStateBuilder.java| 7 +- .../instance/InstanceAssignmentDriver.java | 12 +- .../instance/InstanceTagPoolSelector.java | 1 - .../helix/core/minion/ClusterInfoProvider.java | 36 +-- .../core/relocation/RealtimeSegmentRelocator.java | 8 +- .../helix/core/retention/RetentionManager.java | 2 +- ...PinotInstanceAssignmentRestletResourceTest.java | 22 +- .../api/PinotTableRestletResourceTest.java | 71 ++ .../pinot/controller/helix/ControllerTest.java | 26 +-
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #4870: Add getResultColumnName API to AggregationFunction to fetch sql compliant result name
Jackie-Jiang commented on a change in pull request #4870: Add
getResultColumnName API to AggregationFunction to fetch sql compliant result
name
URL: https://github.com/apache/incubator-pinot/pull/4870#discussion_r351507818
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/table/TableResizer.java
##
@@ -57,15 +57,18 @@
int numAggregations = aggregationInfos.size();
int numKeyColumns = numColumns - numAggregations;
+int aggIdx = 0;
Map columnIndexMap = new HashMap<>();
+Map aggregationColumnToFunction = new
HashMap<>();
for (int i = 0; i < numColumns; i++) {
- columnIndexMap.put(dataSchema.getColumnName(i), i);
-}
-
-Map aggregationColumnToInfo = new HashMap<>();
-for (AggregationInfo aggregationInfo : aggregationInfos) {
- String aggregationColumn =
AggregationFunctionUtils.getAggregationColumnName(aggregationInfo);
- aggregationColumnToInfo.put(aggregationColumn, aggregationInfo);
+ String columnName = dataSchema.getColumnName(i).toLowerCase();
+ columnIndexMap.put(columnName, i);
+ if (i >= numKeyColumns) {
+AggregationFunction aggregationFunction =
Review comment:
Please add some comments here (key columns are in front of aggregation
columns).
Also, `aggIdx === i - numKeyColumns`, so you don't need to maintain another
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] Jackie-Jiang commented on a change in pull request #4870: Add getResultColumnName API to AggregationFunction to fetch sql compliant result name
Jackie-Jiang commented on a change in pull request #4870: Add
getResultColumnName API to AggregationFunction to fetch sql compliant result
name
URL: https://github.com/apache/incubator-pinot/pull/4870#discussion_r351506290
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/table/TableResizer.java
##
@@ -57,15 +57,18 @@
int numAggregations = aggregationInfos.size();
int numKeyColumns = numColumns - numAggregations;
+int aggIdx = 0;
Map columnIndexMap = new HashMap<>();
+Map aggregationColumnToFunction = new
HashMap<>();
for (int i = 0; i < numColumns; i++) {
- columnIndexMap.put(dataSchema.getColumnName(i), i);
-}
-
-Map aggregationColumnToInfo = new HashMap<>();
-for (AggregationInfo aggregationInfo : aggregationInfos) {
- String aggregationColumn =
AggregationFunctionUtils.getAggregationColumnName(aggregationInfo);
- aggregationColumnToInfo.put(aggregationColumn, aggregationInfo);
+ String columnName = dataSchema.getColumnName(i).toLowerCase();
Review comment:
This is not correct as `column` and `COLUMN` should be treated as two
different columns (case sensitive)
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] Jackie-Jiang merged pull request #4865: Misc clean up for PinotHelixResourceManager
Jackie-Jiang merged pull request #4865: Misc clean up for PinotHelixResourceManager URL: https://github.com/apache/incubator-pinot/pull/4865 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 (03ae23c -> fdfe8a2)
This is an automated email from the ASF dual-hosted git repository. jackie pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. from 03ae23c [TE] Check if snapshot exists before attaching file (#4867) add fdfe8a2 Misc clean up for PinotHelixResourceManager (#4865) No new revisions were added by this update. Summary of changes: .../pinot/common/config/TableNameBuilder.java | 2 +- .../api/resources/PinotSegmentRestletResource.java | 2 +- .../PinotSegmentUploadDownloadRestletResource.java | 4 +- .../helix/core/PinotHelixResourceManager.java | 215 + .../helix/core/minion/ClusterInfoProvider.java | 34 ++-- .../core/relocation/RealtimeSegmentRelocator.java | 2 +- .../helix/core/retention/RetentionManager.java | 2 +- 7 files changed, 113 insertions(+), 148 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] kishoreg closed pull request #4778: Decoupling Server instance name from physical host and port
kishoreg closed pull request #4778: Decoupling Server instance name from physical host and port URL: https://github.com/apache/incubator-pinot/pull/4778 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 issue #4778: Decoupling Server instance name from physical host and port
kishoreg commented on issue #4778: Decoupling Server instance name from physical host and port URL: https://github.com/apache/incubator-pinot/pull/4778#issuecomment-559246508 @Jackie-Jiang fixed this 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 #4865: Misc clean up for PinotHelixResourceManager
codecov-io edited a comment on issue #4865: Misc clean up for PinotHelixResourceManager URL: https://github.com/apache/incubator-pinot/pull/4865#issuecomment-558932565 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4865?src=pr=h1) Report > :exclamation: No coverage uploaded for pull request base (`master@03ae23c`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit). > The diff coverage is `75%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4865?src=pr=tree) ```diff @@Coverage Diff@@ ## master#4865 +/- ## = Coverage ? 57.81% Complexity?4 = Files ? 1200 Lines ?64659 Branches ? 9390 = Hits ?37380 Misses?24508 Partials ? 2771 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4865?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...g/apache/pinot/common/config/TableNameBuilder.java](https://codecov.io/gh/apache/incubator-pinot/pull/4865/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL1RhYmxlTmFtZUJ1aWxkZXIuamF2YQ==) | `100% <ø> (ø)` | `0 <0> (?)` | | | [...ces/PinotSegmentUploadDownloadRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/4865/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90U2VnbWVudFVwbG9hZERvd25sb2FkUmVzdGxldFJlc291cmNlLmphdmE=) | `60.47% <ø> (ø)` | `0 <0> (?)` | | | [...ler/api/resources/PinotSegmentRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/4865/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90U2VnbWVudFJlc3RsZXRSZXNvdXJjZS5qYXZh) | `20% <0%> (ø)` | `0 <0> (?)` | | | [...troller/helix/core/retention/RetentionManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4865/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JldGVudGlvbi9SZXRlbnRpb25NYW5hZ2VyLmphdmE=) | `75% <100%> (ø)` | `0 <0> (?)` | | | [...elix/core/relocation/RealtimeSegmentRelocator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4865/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlbG9jYXRpb24vUmVhbHRpbWVTZWdtZW50UmVsb2NhdG9yLmphdmE=) | `40% <100%> (ø)` | `0 <0> (?)` | | | [...troller/helix/core/minion/ClusterInfoProvider.java](https://codecov.io/gh/apache/incubator-pinot/pull/4865/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9DbHVzdGVySW5mb1Byb3ZpZGVyLmphdmE=) | `35.71% <33.33%> (ø)` | `0 <0> (?)` | | | [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4865/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `62.13% <78.26%> (ø)` | `0 <0> (?)` | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4865?src=pr=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/4865?src=pr=footer). Last update [03ae23c...e6e4805](https://codecov.io/gh/apache/incubator-pinot/pull/4865?src=pr=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] kishoreg commented on issue #4870: Add getResultColumnName API to AggregationFunction to fetch sql compliant result name
kishoreg commented on issue #4870: Add getResultColumnName API to AggregationFunction to fetch sql compliant result name URL: https://github.com/apache/incubator-pinot/pull/4870#issuecomment-559244561 what functions in selections and group by. 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 issue #4870: Add getResultColumnName API to AggregationFunction to fetch sql compliant result name
kishoreg commented on issue #4870: Add getResultColumnName API to AggregationFunction to fetch sql compliant result name URL: https://github.com/apache/incubator-pinot/pull/4870#issuecomment-559244374 This is good to have for now since we don't support aliasing. But this should be typically solved at a very high level using a mapping table. Let's make sure that this will not make it difficult to implement aliasing later. 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 opened a new pull request #4870: Add getResultColumnName API to AggregationFunction to fetch sql compliant result name
npawar opened a new pull request #4870: Add getResultColumnName API to AggregationFunction to fetch sql compliant result name URL: https://github.com/apache/incubator-pinot/pull/4870 PQL converts column name as "AVG(foo)" -> "avg_foo". In the SQL style, this would have been "avg(foo)". Adding a new API to get the new result name. Enhanced tests to check the right name is received at the ResultTable in the broker results This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. 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 pinot-ingestion-refactor updated (94274a0 -> 5bc3ebb)
This is an automated email from the ASF dual-hosted git repository.
xiangfu pushed a change to branch pinot-ingestion-refactor
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git.
discard 94274a0 Remove pinot spark Readme.md
add 5bc3ebb Update Readme to refer to docs
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 (94274a0)
\
N -- N -- N refs/heads/pinot-ingestion-refactor (5bc3ebb)
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-ingestion-jobs/pinot-hadoop/README.md| 55 +-
.../pinot-spark}/README.md | 6 ++-
2 files changed, 5 insertions(+), 56 deletions(-)
copy {pinot-connectors/pinot-connector-kafka-base =>
pinot-ingestion-jobs/pinot-spark}/README.md (77%)
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] kishoreg opened a new pull request #4869: Adding pinot-spi module and moving record reader interface
kishoreg opened a new pull request #4869: Adding pinot-spi module and moving record reader interface URL: https://github.com/apache/incubator-pinot/pull/4869 Added pinot-spi module. I based this on the changes made it #4866. Mostly involves moving the classes around. Schema moves to pinot-spi along with this. It was not as invasive as I had thought. Please review. 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 #4866: Removing segment generation config from RecordReader interface
kishoreg commented on a change in pull request #4866: Removing segment
generation config from RecordReader interface
URL: https://github.com/apache/incubator-pinot/pull/4866#discussion_r351476801
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/readers/RecordReader.java
##
@@ -34,10 +34,16 @@
public interface RecordReader extends Closeable {
/**
- * Initializes the record reader when needed
+ * initializing recordreader with inputpath, schema and recordreader config.
+ * The implementation can chose to ignore one or more of these parameters
and handle null gracefully
+ *
+ * @param inputPath absolute path to the file/directory
+ * @param schema Pinot Schema associated with the table
+ * @param recordReaderConfig config for the reader specific to the format.
e.g. delimiter for csv format etc
+ * @throws Exception if the arguments are invalid
*/
- void init(SegmentGeneratorConfig segmentGeneratorConfig)
- throws IOException;
+ void init(@Nullable String inputPath, @Nullable Schema schema, @Nullable
RecordReaderConfig recordReaderConfig)
Review comment:
I could not conclude that looking at the existing implementations.
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 #4866: Removing segment generation config from RecordReader interface
kishoreg commented on a change in pull request #4866: Removing segment
generation config from RecordReader interface
URL: https://github.com/apache/incubator-pinot/pull/4866#discussion_r351476621
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/readers/RecordReader.java
##
@@ -34,10 +34,16 @@
public interface RecordReader extends Closeable {
/**
- * Initializes the record reader when needed
+ * initializing recordreader with inputpath, schema and recordreader config.
+ * The implementation can chose to ignore one or more of these parameters
and handle null gracefully
+ *
+ * @param inputPath absolute path to the file/directory
+ * @param schema Pinot Schema associated with the table
+ * @param recordReaderConfig config for the reader specific to the format.
e.g. delimiter for csv format etc
+ * @throws Exception if the arguments are invalid
*/
- void init(SegmentGeneratorConfig segmentGeneratorConfig)
- throws IOException;
+ void init(@Nullable String inputPath, @Nullable Schema schema, @Nullable
RecordReaderConfig recordReaderConfig)
Review comment:
> This config is for record readers with extra settings (e.g. CSV
delimiter), which can be null. Input path and schema are required for all
record readers
Not really, some of them take GenericRows. There are too many
implementations for recordReader: used in testing, segment conversion, Minion
etc. Most of the readers get what they want in their constructors today.
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 #4866: Removing segment generation config from RecordReader interface
kishoreg commented on a change in pull request #4866: Removing segment
generation config from RecordReader interface
URL: https://github.com/apache/incubator-pinot/pull/4866#discussion_r351475714
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/readers/RecordReader.java
##
@@ -34,10 +34,16 @@
public interface RecordReader extends Closeable {
/**
- * Initializes the record reader when needed
+ * initializing recordreader with inputpath, schema and recordreader config.
+ * The implementation can chose to ignore one or more of these parameters
and handle null gracefully
+ *
+ * @param inputPath absolute path to the file/directory
+ * @param schema Pinot Schema associated with the table
+ * @param recordReaderConfig config for the reader specific to the format.
e.g. delimiter for csv format etc
+ * @throws Exception if the arguments are invalid
*/
- void init(SegmentGeneratorConfig segmentGeneratorConfig)
- throws IOException;
+ void init(@Nullable String inputPath, @Nullable Schema schema, @Nullable
RecordReaderConfig recordReaderConfig)
Review comment:
I thought about it. Not all RecordReader need that. Ideally, we want to
pass a PinotFS interface wrapper but want to avoid making too many changes in
this PR. My first goal is to decouple the modules without adding too much
functionality/changing existing classes. We will have a better idea when we
move the interfaces to spi module.
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 pinot-ingestion-refactor updated (3601d26 -> 94274a0)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch pinot-ingestion-refactor in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. from 3601d26 Adding docs for pinot spark usage add 94274a0 Remove pinot spark Readme.md No new revisions were added by this update. Summary of changes: pinot-ingestion-jobs/pinot-spark/README.md | 93 -- 1 file changed, 93 deletions(-) delete mode 100644 pinot-ingestion-jobs/pinot-spark/README.md - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #4866: Removing segment generation config from RecordReader interface
Jackie-Jiang commented on a change in pull request #4866: Removing segment
generation config from RecordReader interface
URL: https://github.com/apache/incubator-pinot/pull/4866#discussion_r351473176
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/readers/RecordReader.java
##
@@ -34,10 +34,16 @@
public interface RecordReader extends Closeable {
/**
- * Initializes the record reader when needed
+ * initializing recordreader with inputpath, schema and recordreader config.
+ * The implementation can chose to ignore one or more of these parameters
and handle null gracefully
+ *
+ * @param inputPath absolute path to the file/directory
+ * @param schema Pinot Schema associated with the table
+ * @param recordReaderConfig config for the reader specific to the format.
e.g. delimiter for csv format etc
+ * @throws Exception if the arguments are invalid
*/
- void init(SegmentGeneratorConfig segmentGeneratorConfig)
- throws IOException;
+ void init(@Nullable String inputPath, @Nullable Schema schema, @Nullable
RecordReaderConfig recordReaderConfig)
Review comment:
Should we make inputPath and schema mandatory?
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 create-pinot-spi created (now 5942774)
This is an automated email from the ASF dual-hosted git repository. kishoreg pushed a change to branch create-pinot-spi in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. at 5942774 Removing segment generation config from RecordReader interface No new revisions were added by this update. - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #4866: Removing segment generation config from RecordReader interface
Jackie-Jiang commented on a change in pull request #4866: Removing segment
generation config from RecordReader interface
URL: https://github.com/apache/incubator-pinot/pull/4866#discussion_r351472583
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/readers/RecordReader.java
##
@@ -34,10 +34,16 @@
public interface RecordReader extends Closeable {
/**
- * Initializes the record reader when needed
+ * initializing recordreader with inputpath, schema and recordreader config.
+ * The implementation can chose to ignore one or more of these parameters
and handle null gracefully
+ *
+ * @param inputPath absolute path to the file/directory
+ * @param schema Pinot Schema associated with the table
+ * @param recordReaderConfig config for the reader specific to the format.
e.g. delimiter for csv format etc
+ * @throws Exception if the arguments are invalid
*/
- void init(SegmentGeneratorConfig segmentGeneratorConfig)
- throws IOException;
+ void init(@Nullable String inputPath, @Nullable Schema schema, @Nullable
RecordReaderConfig recordReaderConfig)
Review comment:
This config is for record readers with extra settings (e.g. CSV delimiter),
which can be null. Input path and schema are required for all record readers
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] Jackie-Jiang commented on a change in pull request #4865: Misc clean up for PinotHelixResourceManager
Jackie-Jiang commented on a change in pull request #4865: Misc clean up for
PinotHelixResourceManager
URL: https://github.com/apache/incubator-pinot/pull/4865#discussion_r351468918
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##
@@ -394,44 +387,40 @@ public synchronized PinotResourceManagerResponse
addInstance(@Nonnull Instance i
}
/**
- * Get all Pinot realtime table names.
+ * Get all offline table names.
*
- * @return List of Pinot realtime table names
+ * @return List of offline table names
*/
- @Nonnull
- public List getAllRealtimeTables() {
-List resourceNames = getAllResources();
-Iterator iterator = resourceNames.iterator();
-while (iterator.hasNext()) {
- if (!TableNameBuilder.REALTIME.tableHasTypeSuffix(iterator.next())) {
-iterator.remove();
+ public List getAllOfflineTables() {
+List offlineTableNames = new ArrayList<>();
+for (String resourceName : getAllResources()) {
+ if (TableNameBuilder.OFFLINE.tableHasTypeSuffix(resourceName)) {
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] jackjlli commented on a change in pull request #4865: Misc clean up for PinotHelixResourceManager
jackjlli commented on a change in pull request #4865: Misc clean up for
PinotHelixResourceManager
URL: https://github.com/apache/incubator-pinot/pull/4865#discussion_r351460201
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##
@@ -394,44 +387,40 @@ public synchronized PinotResourceManagerResponse
addInstance(@Nonnull Instance i
}
/**
- * Get all Pinot realtime table names.
+ * Get all offline table names.
*
- * @return List of Pinot realtime table names
+ * @return List of offline table names
*/
- @Nonnull
- public List getAllRealtimeTables() {
-List resourceNames = getAllResources();
-Iterator iterator = resourceNames.iterator();
-while (iterator.hasNext()) {
- if (!TableNameBuilder.REALTIME.tableHasTypeSuffix(iterator.next())) {
-iterator.remove();
+ public List getAllOfflineTables() {
+List offlineTableNames = new ArrayList<>();
+for (String resourceName : getAllResources()) {
+ if (TableNameBuilder.OFFLINE.tableHasTypeSuffix(resourceName)) {
+offlineTableNames.add(resourceName);
}
}
-return resourceNames;
+return offlineTableNames;
}
/**
- * Get all Pinot offline table names
- * @return List of Pinot realtime table names
+ * Get all realtime table names.
+ *
+ * @return List of realtime table names
*/
- @Nonnull
- public List getAllOfflineTables() {
-List resourceNames = getAllResources();
-Iterator iterator = resourceNames.iterator();
-while (iterator.hasNext()) {
- if (!TableNameBuilder.OFFLINE.tableHasTypeSuffix(iterator.next())) {
-iterator.remove();
+ public List getAllRealtimeTables() {
+List realtimeTableNames = new ArrayList<>();
+for (String resourceName : getAllResources()) {
+ if (TableNameBuilder.REALTIME.tableHasTypeSuffix(resourceName)) {
Review comment:
Same here. `TableNameBuilder.isRealtimeTableResource(resourceName)` might be
better.
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] jackjlli commented on a change in pull request #4865: Misc clean up for PinotHelixResourceManager
jackjlli commented on a change in pull request #4865: Misc clean up for
PinotHelixResourceManager
URL: https://github.com/apache/incubator-pinot/pull/4865#discussion_r351459857
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##
@@ -394,44 +387,40 @@ public synchronized PinotResourceManagerResponse
addInstance(@Nonnull Instance i
}
/**
- * Get all Pinot realtime table names.
+ * Get all offline table names.
*
- * @return List of Pinot realtime table names
+ * @return List of offline table names
*/
- @Nonnull
- public List getAllRealtimeTables() {
-List resourceNames = getAllResources();
-Iterator iterator = resourceNames.iterator();
-while (iterator.hasNext()) {
- if (!TableNameBuilder.REALTIME.tableHasTypeSuffix(iterator.next())) {
-iterator.remove();
+ public List getAllOfflineTables() {
+List offlineTableNames = new ArrayList<>();
+for (String resourceName : getAllResources()) {
+ if (TableNameBuilder.OFFLINE.tableHasTypeSuffix(resourceName)) {
Review comment:
I prefer keeping this method `tableHasTypeSuffix` within TableNameBuilder
and marked as private access method.
And this line be replaced by
`TableNameBuilder.isOfflineTableResource(resourceName)`.
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] harleyjj opened a new pull request #4868: [TE] frontend - harleyjj/bugs - Dont filter zeroes from bounds and di…
harleyjj opened a new pull request #4868: [TE] frontend - harleyjj/bugs - Dont filter zeroes from bounds and di… URL: https://github.com/apache/incubator-pinot/pull/4868 …splay error message on create alert 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] akshayrai merged pull request #4867: [TE] Check if snapshot exists before attaching file
akshayrai merged pull request #4867: [TE] Check if snapshot exists before attaching file URL: https://github.com/apache/incubator-pinot/pull/4867 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 (05c89fc -> 03ae23c)
This is an automated email from the ASF dual-hosted git repository. akshayrai09 pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. from 05c89fc Enhance TagNameUtils to handle the TagOverrideConfig (#4859) add 03ae23c [TE] Check if snapshot exists before attaching file (#4867) No new revisions were added by this update. Summary of changes: .../pinot/thirdeye/notification/commons/ThirdEyeJiraClient.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #4800: Pinot Ingestion Jobs Refactor with Pinot Spark Support
mcvsubbu commented on a change in pull request #4800: Pinot Ingestion Jobs Refactor with Pinot Spark Support URL: https://github.com/apache/incubator-pinot/pull/4800#discussion_r351424357 ## File path: pinot-ingestion-jobs/pinot-spark/README.md ## @@ -0,0 +1,93 @@ + +# Pinot Spark Review comment: yup, that also works as long as contents are in one place. 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] akshayrai opened a new pull request #4867: [TE] Check if snapshot exists before attaching file
akshayrai opened a new pull request #4867: [TE] Check if snapshot exists before attaching file URL: https://github.com/apache/incubator-pinot/pull/4867 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] mcvsubbu commented on a change in pull request #4865: Misc clean up for PinotHelixResourceManager
mcvsubbu commented on a change in pull request #4865: Misc clean up for
PinotHelixResourceManager
URL: https://github.com/apache/incubator-pinot/pull/4865#discussion_r351419620
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##
@@ -1321,19 +1304,15 @@ public void updateIndexingConfigFor(String tableName,
TableType type, IndexingCo
setExistingTableConfig(tableConfig);
}
- private void handleBrokerResource(@Nonnull final String tableName, @Nonnull
final List brokersForTenant) {
-LOGGER.info("Updating BrokerResource IdealState for table: {}", tableName);
-HelixHelper
-.updateIdealState(_helixZkManager, Helix.BROKER_RESOURCE_INSTANCE, new
Function() {
- @Override
- public IdealState apply(@Nullable IdealState idealState) {
-Preconditions.checkNotNull(idealState);
-for (String broker : brokersForTenant) {
- idealState.setPartitionState(tableName, broker,
BrokerOnlineOfflineStateModel.ONLINE);
-}
-return idealState;
- }
-}, RetryPolicies.exponentialBackoffRetryPolicy(5, 500L, 2.0f));
+ private void handleBrokerResource(String tableNameWithType, List
brokersForTenant) {
Review comment:
suggest rename to enableTableInBrokers(). Or, updateBrokerResource() but
that is more vague.
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 #4800: Pinot Ingestion Jobs Refactor with Pinot Spark Support
kishoreg commented on a change in pull request #4800: Pinot Ingestion Jobs Refactor with Pinot Spark Support URL: https://github.com/apache/incubator-pinot/pull/4800#discussion_r351421621 ## File path: pinot-ingestion-jobs/pinot-spark/README.md ## @@ -0,0 +1,93 @@ + +# Pinot Spark Review comment: Let keep the README. You can point the README to documentation link. 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] mcvsubbu commented on a change in pull request #4800: Pinot Ingestion Jobs Refactor with Pinot Spark Support
mcvsubbu commented on a change in pull request #4800: Pinot Ingestion Jobs Refactor with Pinot Spark Support URL: https://github.com/apache/incubator-pinot/pull/4800#discussion_r351415964 ## File path: docs/pinot_hadoop.rst ## @@ -87,6 +87,70 @@ You can then use the SegmentTarPush job to push segments via the controller REST hadoop jar pinot-hadoop--SNAPSHOT-shaded.jar SegmentTarPush job.properties + +Creating Pinot segments using Spark Review comment: Thanks so much for adding this. 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] mcvsubbu commented on a change in pull request #4800: Pinot Ingestion Jobs Refactor with Pinot Spark Support
mcvsubbu commented on a change in pull request #4800: Pinot Ingestion Jobs Refactor with Pinot Spark Support URL: https://github.com/apache/incubator-pinot/pull/4800#discussion_r351415628 ## File path: pinot-ingestion-jobs/pinot-spark/README.md ## @@ -0,0 +1,93 @@ + +# Pinot Spark Review comment: Can we please remove README.md? It is best if documentation is in one place. 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] codecov-io edited a comment on issue #4860: Add string udfs
codecov-io edited a comment on issue #4860: Add string udfs URL: https://github.com/apache/incubator-pinot/pull/4860#issuecomment-558817068 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4860?src=pr=h1) Report > Merging [#4860](https://codecov.io/gh/apache/incubator-pinot/pull/4860?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/05c89fca6744b6f5d8ffd0e1c19c8ed0e28d0ae6?src=pr=desc) will **decrease** coverage by `7.53%`. > The diff coverage is `85.71%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4860?src=pr=tree) ```diff @@ Coverage Diff @@ ## master #4860 +/- ## === - Coverage 57.83% 50.3% -7.54% Complexity4 4 === Files 12001201 +1 Lines 64663 64691 +28 Branches 93899393 +4 === - Hits 37401 32540-4861 - Misses24491 29637+5146 + Partials 27712514 -257 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4860?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4860/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `65.51% <100%> (-4.13%)` | `0 <0> (ø)` | | | [...m/function/SingleParamStringTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4860/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vU2luZ2xlUGFyYW1TdHJpbmdUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `84.61% <84.61%> (ø)` | `0 <0> (?)` | | | [...g/apache/pinot/sql/parsers/CalciteSqlCompiler.java](https://codecov.io/gh/apache/incubator-pinot/pull/4860/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvcGFyc2Vycy9DYWxjaXRlU3FsQ29tcGlsZXIuamF2YQ==) | `0% <0%> (-100%)` | `0% <0%> (ø)` | | | [...pache/pinot/core/realtime/stream/MessageBatch.java](https://codecov.io/gh/apache/incubator-pinot/pull/4860/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9zdHJlYW0vTWVzc2FnZUJhdGNoLmphdmE=) | `0% <0%> (-100%)` | `0% <0%> (ø)` | | | [...che/pinot/hadoop/io/CombineAvroKeyInputFormat.java](https://codecov.io/gh/apache/incubator-pinot/pull/4860/diff?src=pr=tree#diff-cGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3AvaW8vQ29tYmluZUF2cm9LZXlJbnB1dEZvcm1hdC5qYXZh) | `0% <0%> (-100%)` | `0% <0%> (ø)` | | | [...starter/helix/DefaultHelixStarterServerConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/4860/diff?src=pr=tree#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9EZWZhdWx0SGVsaXhTdGFydGVyU2VydmVyQ29uZmlnLmphdmE=) | `0% <0%> (-96.16%)` | `0% <0%> (ø)` | | | [...re/data/manager/config/TableDataManagerConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/4860/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvY29uZmlnL1RhYmxlRGF0YU1hbmFnZXJDb25maWcuamF2YQ==) | `0% <0%> (-94.45%)` | `0% <0%> (ø)` | | | [...converter/stats/RealtimeSegmentStatsContainer.java](https://codecov.io/gh/apache/incubator-pinot/pull/4860/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9jb252ZXJ0ZXIvc3RhdHMvUmVhbHRpbWVTZWdtZW50U3RhdHNDb250YWluZXIuamF2YQ==) | `0% <0%> (-94.45%)` | `0% <0%> (ø)` | | | [...ot/hadoop/job/partitioners/GenericPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/4860/diff?src=pr=tree#diff-cGlub3QtaGFkb29wL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9oYWRvb3Avam9iL3BhcnRpdGlvbmVycy9HZW5lcmljUGFydGl0aW9uZXIuamF2YQ==) | `0% <0%> (-93.34%)` | `0% <0%> (ø)` | | | [...esthandler/ConnectionPoolBrokerRequestHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/4860/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvQ29ubmVjdGlvblBvb2xCcm9rZXJSZXF1ZXN0SGFuZGxlci5qYXZh) | `0% <0%> (-88.83%)` | `0% <0%> (ø)` | | | ... and [279 more](https://codecov.io/gh/apache/incubator-pinot/pull/4860/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4860?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute
[GitHub] [incubator-pinot] codecov-io commented on issue #4866: Removing segment generation config from RecordReader interface
codecov-io commented on issue #4866: Removing segment generation config from RecordReader interface URL: https://github.com/apache/incubator-pinot/pull/4866#issuecomment-558984119 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4866?src=pr=h1) Report > Merging [#4866](https://codecov.io/gh/apache/incubator-pinot/pull/4866?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/05c89fca6744b6f5d8ffd0e1c19c8ed0e28d0ae6?src=pr=desc) will **decrease** coverage by `7.52%`. > The diff coverage is `40%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4866?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4866 +/- ## - Coverage 57.83% 50.31% -7.53% Complexity44 Files 1200 1200 Lines 6466364663 Branches 9389 9389 - Hits 3740132535-4866 - Misses2449129614+5123 + Partials 2771 2514 -257 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4866?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...inot/core/data/readers/GenericRowRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3JlYWRlcnMvR2VuZXJpY1Jvd1JlY29yZFJlYWRlci5qYXZh) | `93.33% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...ache/pinot/core/data/readers/JSONRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3JlYWRlcnMvSlNPTlJlY29yZFJlYWRlci5qYXZh) | `86.66% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...pache/pinot/core/data/readers/CSVRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3JlYWRlcnMvQ1NWUmVjb3JkUmVhZGVyLmphdmE=) | `69.81% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [.../pinot/core/minion/segment/MapperRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9taW5pb24vc2VnbWVudC9NYXBwZXJSZWNvcmRSZWFkZXIuamF2YQ==) | `81.08% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...pinot/core/minion/segment/ReducerRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9taW5pb24vc2VnbWVudC9SZWR1Y2VyUmVjb3JkUmVhZGVyLmphdmE=) | `90.19% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...ealtime/converter/RealtimeSegmentRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9jb252ZXJ0ZXIvUmVhbHRpbWVTZWdtZW50UmVjb3JkUmVhZGVyLmphdmE=) | `0% <ø> (-58.34%)` | `0 <0> (ø)` | | | [...va/org/apache/pinot/core/minion/SegmentPurger.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9taW5pb24vU2VnbWVudFB1cmdlci5qYXZh) | `77.41% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...ache/pinot/core/minion/BackfillDateTimeColumn.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9taW5pb24vQmFja2ZpbGxEYXRlVGltZUNvbHVtbi5qYXZh) | `56.25% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...ache/pinot/core/data/readers/AvroRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3JlYWRlcnMvQXZyb1JlY29yZFJlYWRlci5qYXZh) | `86.2% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...data/readers/MultiplePinotSegmentRecordReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3JlYWRlcnMvTXVsdGlwbGVQaW5vdFNlZ21lbnRSZWNvcmRSZWFkZXIuamF2YQ==) | `72.22% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: | | ... and [289 more](https://codecov.io/gh/apache/incubator-pinot/pull/4866/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4866?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`,
