Re: [PR] Compute all groups for group by queries with only filtered aggregations [pinot]

yashmayya commented on code in PR #14211: URL: https://github.com/apache/pinot/pull/14211#discussion_r1797582617 ## pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java: ## @@ -458,6 +458,15 @@ public static class QueryOptionKey { // executed in an U

Re: [PR] Compute all groups for group by queries with only filtered aggregations [pinot]

yashmayya commented on code in PR #14211: URL: https://github.com/apache/pinot/pull/14211#discussion_r1797545799 ## pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java: ## @@ -458,6 +458,15 @@ public static class QueryOptionKey { // executed in an U

Re: [PR] Compute all groups for group by queries with only filtered aggregations [pinot]

yashmayya commented on code in PR #14211: URL: https://github.com/apache/pinot/pull/14211#discussion_r1797545799 ## pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java: ## @@ -458,6 +458,15 @@ public static class QueryOptionKey { // executed in an U

Re: [PR] Fix default column handling when forward index is disabled [pinot]

codecov-commenter commented on PR #14215: URL: https://github.com/apache/pinot/pull/14215#issuecomment-2408304815 ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/14215?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&u

Re: [PR] Fix race-conditions in IdealStateGroupCommit [pinot]

dinoocch commented on code in PR #14214: URL: https://github.com/apache/pinot/pull/14214#discussion_r1797525051 ## pinot-common/src/main/java/org/apache/pinot/common/utils/helix/IdealStateGroupCommit.java: ## @@ -124,34 +124,27 @@ public IdealState commit(HelixManager helixManag

[PR] Fix default column handling when forward index is disabled [pinot]

Jackie-Jiang opened a new pull request, #14215: URL: https://github.com/apache/pinot/pull/14215 This was introduced in #14150 where `_forwardIndexDisabledColumns` is no longer extracted in `IndexLoadingConfig` Fix the behavior in `BaseDefaultColumnHandler` to use the standard way to

Re: [PR] Fix race-conditions in IdealStateGroupCommit [pinot]

jasperjiaguo commented on code in PR #14214: URL: https://github.com/apache/pinot/pull/14214#discussion_r1797515746 ## pinot-common/src/main/java/org/apache/pinot/common/utils/helix/IdealStateGroupCommit.java: ## @@ -124,34 +124,27 @@ public IdealState commit(HelixManager helixM

Re: [PR] Fix race-conditions in IdealStateGroupCommit [pinot]

codecov-commenter commented on PR #14214: URL: https://github.com/apache/pinot/pull/14214#issuecomment-2408286429 ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/14214?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&u

Re: [PR] [DO NOT MERGE - triggering tests] Egalpin/refactor broker request handle request [pinot]

codecov-commenter commented on PR #14213: URL: https://github.com/apache/pinot/pull/14213#issuecomment-2408272469 ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/14213?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&u

[PR] Fix race-conditions in IdealStateGroupCommit [pinot]

dinoocch opened a new pull request, #14214: URL: https://github.com/apache/pinot/pull/14214 This attempts to fix two issues I believe are introduced in https://github.com/apache/pinot/pull/13976/ I think both can cause a table's idealstate to be updated into an unintended state. W

[PR] [DO NOT MERGE - triggering tests] Egalpin/refactor broker request handle request [pinot]

egalpin opened a new pull request, #14213: URL: https://github.com/apache/pinot/pull/14213 Instructions: 1. The PR has to be tagged with at least one of the following labels (*): 1. `feature` 2. `bugfix` 3. `performance` 4. `ui` 5. `backward-incompat` 6.

Re: [PR] Enabling LogicalProject pushdown optimizations to eliminate exchange of unused columns [pinot]

Jackie-Jiang commented on PR #14198: URL: https://github.com/apache/pinot/pull/14198#issuecomment-2408237458 These rules are repeated in multiple places, and rule sets are executed multiple times, introducing extra overhead and currently very hard to manage. Do you see a better way to re-or

Re: [PR] Add Tabled Is Disabled Error [pinot]

Jackie-Jiang commented on code in PR #14199: URL: https://github.com/apache/pinot/pull/14199#discussion_r1797491985 ## pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java: ## @@ -485,6 +490,16 @@ protected BrokerResponse han

Re: [PR] colocated-without-hints [pinot]

Jackie-Jiang commented on code in PR #13943: URL: https://github.com/apache/pinot/pull/13943#discussion_r1797478438 ## pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotImplicitTableHintRule.java: ## @@ -0,0 +1,225 @@ +/** + * Licensed to the Apache Softw

(pinot) branch master updated: Extract common MV ser/de logic into ArraySerDeUtils (#14209)

This is an automated email from the ASF dual-hosted git repository. jackie pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pinot.git The following commit(s) were added to refs/heads/master by this push: new b067ae1924 Extract common MV ser/de logic into Ar

Re: [PR] Extract common MV ser/de logic into ArraySerDeUtils [pinot]

Jackie-Jiang merged PR #14209: URL: https://github.com/apache/pinot/pull/14209 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pinot

Re: [PR] Replace fmpp-maven-plugin with fmpp Ant Task to avoid pulling the vulnerable log4j jars [pinot]

chrajeshbabu commented on PR #14015: URL: https://github.com/apache/pinot/pull/14015#issuecomment-2408179901 Test case failures may not be relevant to the patch. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL

Re: [PR] Enhance `optimizeDictionary` to optionally optimize var-width type cols [pinot]

itschrispeck commented on PR #13994: URL: https://github.com/apache/pinot/pull/13994#issuecomment-2408125471 > Certain settings are impossible, e.g. only apply cardinality based optimization and skip optimization for fixed-length type. I'm not too worried about this limitation, but it would

Re: [PR] Add a flexible json config way [pinot]

lnbest0707-uber commented on code in PR #14200: URL: https://github.com/apache/pinot/pull/14200#discussion_r1797361307 ## pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java: ## @@ -82,6 +84,7 @@ private JsonUtils() { // For querying public static final Str

Re: [PR] Compute all groups for group by queries with only filtered aggregations [pinot]

gortiz commented on code in PR #14211: URL: https://github.com/apache/pinot/pull/14211#discussion_r1797336801 ## pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java: ## @@ -458,6 +458,15 @@ public static class QueryOptionKey { // executed in an Unbo

Re: [PR] Compute all groups for group by queries with only filtered aggregations [pinot]

Jackie-Jiang commented on code in PR #14211: URL: https://github.com/apache/pinot/pull/14211#discussion_r1797326904 ## pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java: ## @@ -458,6 +458,15 @@ public static class QueryOptionKey { // executed in an

Re: [PR] Compute all groups for group by queries with only filtered aggregations [pinot]

yashmayya commented on code in PR #14211: URL: https://github.com/apache/pinot/pull/14211#discussion_r1797322234 ## pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java: ## @@ -458,6 +458,15 @@ public static class QueryOptionKey { // executed in an U

Re: [PR] Compute all groups for group by queries with only filtered aggregations [pinot]

yashmayya commented on code in PR #14211: URL: https://github.com/apache/pinot/pull/14211#discussion_r1797321234 ## pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/MultistageGroupByExecutor.java: ## @@ -241,6 +241,11 @@ private void processAggregate(Tra

Re: [PR] colocated-without-hints [pinot]

Jackie-Jiang commented on PR #13943: URL: https://github.com/apache/pinot/pull/13943#issuecomment-2407950547 @gortiz Yes! I think we can enable it by default -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL abov

Re: [PR] Compute all groups for group by queries with only filtered aggregations [pinot]

Jackie-Jiang commented on code in PR #14211: URL: https://github.com/apache/pinot/pull/14211#discussion_r1797310958 ## pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java: ## @@ -458,6 +458,15 @@ public static class QueryOptionKey { // executed in an

Re: [PR] Compute all groups for group by queries with only filtered aggregations [pinot]

yashmayya commented on code in PR #14211: URL: https://github.com/apache/pinot/pull/14211#discussion_r1797267398 ## pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java: ## @@ -1018,6 +1013,28 @@ public void testMVNumericC

Re: [PR] Allows multiple requests per server per request ID [pinot]

egalpin commented on code in PR #13742: URL: https://github.com/apache/pinot/pull/13742#discussion_r1797269186 ## pinot-core/src/main/java/org/apache/pinot/core/transport/QueryResponse.java: ## @@ -49,12 +50,12 @@ enum Status { /** * Returns the current server responses w

Re: [PR] Multi stage explain imprv [pinot]

codecov-commenter commented on PR #14212: URL: https://github.com/apache/pinot/pull/14212#issuecomment-2407730694 ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/14212?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&u

Re: [PR] Compute all groups for group by queries with only filtered aggregations [pinot]

gortiz commented on code in PR #14211: URL: https://github.com/apache/pinot/pull/14211#discussion_r1797139653 ## pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java: ## @@ -3729,4 +3729,39 @@ public void testSkipIndexes(boo

Re: [PR] Compute all groups for group by queries with only filtered aggregations [pinot]

gortiz commented on code in PR #14211: URL: https://github.com/apache/pinot/pull/14211#discussion_r1797139653 ## pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java: ## @@ -3729,4 +3729,39 @@ public void testSkipIndexes(boo

Re: [PR] Compute all groups for group by queries with only filtered aggregations [pinot]

gortiz commented on code in PR #14211: URL: https://github.com/apache/pinot/pull/14211#discussion_r1797137662 ## pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java: ## @@ -1018,6 +1013,28 @@ public void testMVNumericCast

[PR] Multi stage explain imprv [pinot]

gortiz opened a new pull request, #14212: URL: https://github.com/apache/pinot/pull/14212 This PR fixes 2 non critical but annoying issues in multi-stage: ## Issue 1 Plans from different servers were not correctly merged when segments for each server produced different plans. For e

Re: [PR] PoC for checking Groovy scripts for shell script injection attacks [pinot]

ege-st closed pull request #14103: PoC for checking Groovy scripts for shell script injection attacks URL: https://github.com/apache/pinot/pull/14103 -- 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

Re: [PR] PoC for checking Groovy scripts for shell script injection attacks [pinot]

ege-st commented on PR #14103: URL: https://github.com/apache/pinot/pull/14103#issuecomment-2407546298 Closing. Had to replace with https://github.com/apache/pinot/pull/14197 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub an

Re: [PR] Secure Excecution of Groovy Code [pinot]

ege-st commented on PR #14197: URL: https://github.com/apache/pinot/pull/14197#issuecomment-2407474467 Not quite sure what is happening with the failing tests, looks like it's some CLP tests that are failing. -- This is an automated message from the Apache Git Service. To respond to the m

Re: [PR] Compute all groups for group by queries with only filtered aggregations [pinot]

gortiz commented on code in PR #14211: URL: https://github.com/apache/pinot/pull/14211#discussion_r1796977724 ## pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/MultistageGroupByExecutor.java: ## @@ -241,6 +241,11 @@ private void processAggregate(Transf

Re: [PR] Compute all groups for group by queries with only filtered aggregations [pinot]

codecov-commenter commented on PR #14211: URL: https://github.com/apache/pinot/pull/14211#issuecomment-2407422806 ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/14211?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&u

Re: [PR] Compute all groups for group by queries with only filtered aggregations [pinot]

yashmayya commented on PR #14211: URL: https://github.com/apache/pinot/pull/14211#issuecomment-2407369015 The PR description explains the rationale behind choosing different default behavior for the v1 and v2 query engines here. However, this currently means that there's no way to disable t

[PR] Compute all groups for group by queries with only filtered aggregations [pinot]

yashmayya opened a new pull request, #14211: URL: https://github.com/apache/pinot/pull/14211 - Fixes https://github.com/apache/pinot/issues/12647 and https://github.com/apache/pinot/issues/13982. - Both the above issues have a good description of the problem but the TLDR is that for grou

Re: [PR] colocated-without-hints [pinot]

gortiz commented on PR #13943: URL: https://github.com/apache/pinot/pull/13943#issuecomment-2407330774 @Jackie-Jiang Question: Should we also set `pinot.broker.enable.partition.metadata.manager` to true by default? -- This is an automated message from the Apache Git Service. To respond to

Re: [PR] colocated-without-hints [pinot]

gortiz commented on code in PR #13943: URL: https://github.com/apache/pinot/pull/13943#discussion_r1796900105 ## pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotImplicitTableHintRule.java: ## @@ -0,0 +1,316 @@ +/** + * Licensed to the Apache Software Fo

Re: [I] [proposal] Reuse common expressions in a query [pinot]

gortiz commented on issue #14196: URL: https://github.com/apache/pinot/issues/14196#issuecomment-2407312666 Let's try to trust internet and open comments for everyone :D -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use

Re: [PR] colocated-without-hints [pinot]

gortiz commented on code in PR #13943: URL: https://github.com/apache/pinot/pull/13943#discussion_r1796851649 ## pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotImplicitTableHintRule.java: ## @@ -0,0 +1,316 @@ +/** + * Licensed to the Apache Software Fo

Re: [PR] Replace fmpp-maven-plugin with fmpp Ant Task to avoid pulling the vulnerable log4j jars [pinot]

chrajeshbabu commented on code in PR #14015: URL: https://github.com/apache/pinot/pull/14015#discussion_r1796844838 ## pinot-common/pom.xml: ## @@ -100,22 +100,31 @@ -com.googlecode.fmpp-maven-plugin -fmpp-maven-plugin +org.apache.maven.pl

Re: [PR] Replace fmpp-maven-plugin with fmpp Ant Task to avoid pulling the vulnerable log4j jars [pinot]

chrajeshbabu commented on code in PR #14015: URL: https://github.com/apache/pinot/pull/14015#discussion_r1796821843 ## pinot-common/pom.xml: ## @@ -100,22 +100,31 @@ -com.googlecode.fmpp-maven-plugin -fmpp-maven-plugin +org.apache.maven.pl

Re: [PR] colocated-without-hints [pinot]

gortiz commented on code in PR #13943: URL: https://github.com/apache/pinot/pull/13943#discussion_r1796841573 ## pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotImplicitTableHintRule.java: ## @@ -0,0 +1,316 @@ +/** + * Licensed to the Apache Software Fo

Re: [PR] colocated-without-hints [pinot]

gortiz commented on code in PR #13943: URL: https://github.com/apache/pinot/pull/13943#discussion_r1796836057 ## pinot-query-planner/src/main/java/org/apache/pinot/query/routing/WorkerManager.java: ## @@ -484,79 +507,119 @@ private PartitionTableInfo getPartitionTableInfo(String

Re: [PR] Replace fmpp-maven-plugin with fmpp Ant Task to avoid pulling the vulnerable log4j jars [pinot]

chrajeshbabu commented on code in PR #14015: URL: https://github.com/apache/pinot/pull/14015#discussion_r1796831201 ## pinot-common/pom.xml: ## @@ -100,22 +100,31 @@ -com.googlecode.fmpp-maven-plugin -fmpp-maven-plugin +org.apache.maven.pl

Re: [PR] colocated-without-hints [pinot]

gortiz commented on code in PR #13943: URL: https://github.com/apache/pinot/pull/13943#discussion_r1796828117 ## pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotImplicitTableHintRule.java: ## @@ -0,0 +1,316 @@ +/** + * Licensed to the Apache Software Fo

Re: [PR] colocated-without-hints [pinot]

gortiz commented on code in PR #13943: URL: https://github.com/apache/pinot/pull/13943#discussion_r1796827534 ## pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotImplicitTableHintRule.java: ## @@ -0,0 +1,316 @@ +/** + * Licensed to the Apache Software Fo

Re: [PR] colocated-without-hints [pinot]

gortiz commented on code in PR #13943: URL: https://github.com/apache/pinot/pull/13943#discussion_r1796827534 ## pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotImplicitTableHintRule.java: ## @@ -0,0 +1,316 @@ +/** + * Licensed to the Apache Software Fo

Re: [PR] colocated-without-hints [pinot]

gortiz commented on code in PR #13943: URL: https://github.com/apache/pinot/pull/13943#discussion_r1796825749 ## pom.xml: ## @@ -1444,8 +1444,10 @@ org.immutables -value-annotations +bom Review Comment: Probably not given the processo

Re: [PR] Replace fmpp-maven-plugin with fmpp Ant Task to avoid pulling the vulnerable log4j jars [pinot]

chrajeshbabu commented on code in PR #14015: URL: https://github.com/apache/pinot/pull/14015#discussion_r1796821843 ## pinot-common/pom.xml: ## @@ -100,22 +100,31 @@ -com.googlecode.fmpp-maven-plugin -fmpp-maven-plugin +org.apache.maven.pl

Re: [PR] Speedup dev build. [pinot]

gortiz commented on code in PR #14210: URL: https://github.com/apache/pinot/pull/14210#discussion_r1796787011 ## pinot-controller/pom.xml: ## @@ -121,13 +122,13 @@ -npm ci +npm ci Review Comment: IIRC npm install

Re: [PR] Speedup dev build. [pinot]

codecov-commenter commented on PR #14210: URL: https://github.com/apache/pinot/pull/14210#issuecomment-2406887901 ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/14210?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&u

Re: [PR] colocated-without-hints [pinot]

Jackie-Jiang commented on code in PR #13943: URL: https://github.com/apache/pinot/pull/13943#discussion_r1796556857 ## pinot-query-planner/src/main/java/org/apache/pinot/query/routing/WorkerManager.java: ## @@ -484,79 +507,119 @@ private PartitionTableInfo getPartitionTableInfo(

[PR] Speedup dev build. [pinot]

bziobrowski opened a new pull request, #14210: URL: https://github.com/apache/pinot/pull/14210 This PR speeds up developer build (one with pinot-fastdev profile enabled) by skipping compression and e.g. npm validation. Following build command on my desktop : `mvn -DskipTests=true --als