You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/11/03 13:12:01 UTC

[GitHub] [druid] LakshSingla opened a new pull request #11809: Make subquery IDs more comprehensive

LakshSingla opened a new pull request #11809:
URL: https://github.com/apache/druid/pull/11809


   <!-- Thanks for trying to help us make Apache Druid be the best it can be! Please fill out as much of the following information as is possible (where relevant, and remove it when irrelevant) to help make the intention and scope of this PR clear in order to ease review. -->
   
   <!-- Please read the doc for contribution (https://github.com/apache/druid/blob/master/CONTRIBUTING.md) before making this PR. Also, once you open a PR, please _avoid using force pushes and rebasing_ since these make it difficult for reviewers to see what you've changed in response to their reviews. See [the 'If your pull request shows conflicts with master' section](https://github.com/apache/druid/blob/master/CONTRIBUTING.md#if-your-pull-request-shows-conflicts-with-master) for more details. -->
   
   
   <!-- Replace XXXX with the id of the issue fixed in this PR. Remove this section if there is no corresponding issue. Don't reference the issue in the title of this pull-request. -->
   
   <!-- If you are a committer, follow the PR action item checklist for committers:
   https://github.com/apache/druid/blob/master/dev/committer-instructions.md#pr-and-issue-action-item-checklist-for-committers. -->
   
   ### Description
   
   <!-- Describe the goal of this PR, what problem are you fixing. If there is a corresponding issue (referenced above), it's not necessary to repeat the description here, however, you may choose to keep one summary sentence. -->
   There are 3 types of query IDs - id, subQueryId, sqlQueryId. Currently, whenever a query generates subqueries, the subquery's subQueryId is populated randomly. Also, subquery's Id is not set to the parent query Id. Therefore there is no way of linking the subqueries to the parent query, and one loses the ability to look at end to end view of the query.  
   
   <!-- Describe your patch: what did you change in code? How did you fix the problem? -->
   This PR aims to implement following couple of things:
   1. Populate the subqueries with it's parent's id (and sqlQueryId if present)
   2. Populate the subqueryId such that it forms a hierarchical relationship amongs themselves. For example, if there is a query which launches a subquery, which in turn launches a couple of subqueries, then the ids and subQueryIds should have following structure.
   ```
   Parent Query: { queryId: "my-query-id", subQueryId: null }
       Sub Query: { queryId: "my-query-id", subQueryId: "1" }
           Sub Sub Query: { queryId: "my-query-id", subQueryId: "1-1" }
           Sub Sub Query: { queryId: "my-query-id", subQueryId: "1-2" }
   ```
   Currently this is the implementation of the subQueryId generation. Each of the subquery generated is assigned the ID of the level that datasource resides in the original query.
   1. In case of a subquery which needs to be inlined: It will get assigned the nesting level of the Subquery Datasource under the original datasource where it resides.
   2. In case of an union query: The runner will invoke the query with the individual datasources where subquery Id is assigned to the nesting level of these datasources.
   <!-- If there are several relatively logically separate changes in this PR, create a mini-section for each of them. For example: -->
   
   <!--
   In each section, please describe design decisions made, including:
    - Choice of algorithms
    - Behavioral aspects. What configuration values are acceptable? How are corner cases and error conditions handled, such as when there are insufficient resources?
    - Class organization and design (how the logic is split between classes, inheritance, composition, design patterns)
    - Method organization and design (how the logic is split between methods, parameters and return types)
    - Naming (class, method, API, configuration, HTTP endpoint, names of emitted metrics)
   -->
   
   
   <!-- It's good to describe an alternative design (or mention an alternative name) for every design (or naming) decision point and compare the alternatives with the designs that you've implemented (or the names you've chosen) to highlight the advantages of the chosen designs and names. -->
   
   <!-- If there was a discussion of the design of the feature implemented in this PR elsewhere (e. g. a "Proposal" issue, any other issue, or a thread in the development mailing list), link to that discussion from this PR description and explain what have changed in your final design compared to your original proposal or the consensus version in the end of the discussion. If something hasn't changed since the original discussion, you can omit a detailed discussion of those aspects of the design here, perhaps apart from brief mentioning for the sake of readability of this PR description. -->
   
   <!-- Some of the aspects mentioned above may be omitted for simple and small changes. -->
   
   <hr>
   
   ##### Key changes/added classes in this PR
   `UnionQueryRunner` and `ClientQuerySegmentWalker::inlineIfNecessary` methods populated the `subQueryIds` using the method `Query::withDefaultSubqueryId`. This has been replaced to populate the `subQueryId` based on the parent query Id's `subQueryId`. 
   
   <hr>
   
   <!-- Check the items by putting "x" in the brackets for the done things. Not all of these items apply to every PR. Remove the items which are not done or not relevant to the PR. None of the items from the checklist below are strictly necessary, but it would be very helpful if you at least self-review the PR. -->
   
   This PR has:
   - [ ] been self-reviewed.
      - [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
   - [ ] added documentation for new or modified features or behaviors.
   - [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
   - [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
   - [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
   - [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
   - [ ] added integration tests.
   - [ ] been tested in a test Druid cluster.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 closed pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 closed pull request #11809:
URL: https://github.com/apache/druid/pull/11809


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 closed pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 closed pull request #11809:
URL: https://github.com/apache/druid/pull/11809






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 closed pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 closed pull request #11809:
URL: https://github.com/apache/druid/pull/11809


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] FrankChen021 commented on pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on pull request #11809:
URL: https://github.com/apache/druid/pull/11809#issuecomment-947448538


   Good idea. I think dot-sparated format for subQueryId might be more readable, and this format have been widely used in many distributed tracing systems.
   
   Take your example, it would be
   
   ```
   Parent Query: { queryId: "my-query-id", subQueryId: null }
       Sub Query: { queryId: "my-query-id", subQueryId: "1" }
           Sub Sub Query: { queryId: "my-query-id", subQueryId: "1.1" }
           Sub Sub Query: { queryId: "my-query-id", subQueryId: "1.2" }
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on a change in pull request #11809:
URL: https://github.com/apache/druid/pull/11809#discussion_r740761353



##########
File path: processing/src/main/java/org/apache/druid/query/UnionQueryRunner.java
##########
@@ -93,4 +102,25 @@ public UnionQueryRunner(
     }
   }
 
+  /**
+   * Appends and returns the name and the position of the individual datasource in the union with the parent query id
+   * if preseent
+   *
+   * @param parentSubqueryId The subquery Id of the parent query which is generating this subquery
+   * @param dataSourceName   Name of the datasource for which the UnionRunner is running
+   * @param dataSourceIndex  Position of the datasource for which the UnionRunner is running
+   * @return Subquery Id which needs to be populated
+   */
+  private String generateSubqueryId(String parentSubqueryId, String dataSourceName, int dataSourceIndex)
+  {
+    final String DELIMITER = "-";
+    List<String> arr = new ArrayList<>();

Review comment:
       do we need an array? 

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -147,15 +151,23 @@ public ClientQuerySegmentWalker(
 
     // transform TableDataSource to GlobalTableDataSource when eligible
     // before further transformation to potentially inline
-    final DataSource freeTradeDataSource = globalizeIfPossible(query.getDataSource());
+
+    // Populate the subquery ids of the subquery id present in the main query
+    Query<T> newQuery = query.withDataSource(generateSubqueryIds(query.getDataSource(), "1"));

Review comment:
       ```suggestion
       Query<T> queryWithSubqueryIds = query.withDataSource(generateSubqueryIds(query.getDataSource(), "1"));
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 closed pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 closed pull request #11809:
URL: https://github.com/apache/druid/pull/11809


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] LakshSingla commented on a change in pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
LakshSingla commented on a change in pull request #11809:
URL: https://github.com/apache/druid/pull/11809#discussion_r741574999



##########
File path: processing/src/main/java/org/apache/druid/query/UnionQueryRunner.java
##########
@@ -93,4 +102,25 @@ public UnionQueryRunner(
     }
   }
 
+  /**
+   * Appends and returns the name and the position of the individual datasource in the union with the parent query id
+   * if preseent
+   *
+   * @param parentSubqueryId The subquery Id of the parent query which is generating this subquery
+   * @param dataSourceName   Name of the datasource for which the UnionRunner is running
+   * @param dataSourceIndex  Position of the datasource for which the UnionRunner is running
+   * @return Subquery Id which needs to be populated
+   */
+  private String generateSubqueryId(String parentSubqueryId, String dataSourceName, int dataSourceIndex)
+  {
+    final String DELIMITER = "-";
+    List<String> arr = new ArrayList<>();

Review comment:
       Updated the code, was leftover from my previous implementation.

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));

Review comment:
       The `Query#withSubqueryId()` method returns a new subquery with modified context, as does `Datasource#withChildren`. In order to actually modify the subquery id, we would have to go in a bottom's up manner (from the innermost datasource) and replace the child of the parent datasource, and then return this new parent datasource to be replaced in it's own parent datasource and so forth. I failed to come up with a way of doing that in a loop.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on a change in pull request #11809:
URL: https://github.com/apache/druid/pull/11809#discussion_r744937413



##########
File path: processing/src/main/java/org/apache/druid/query/UnionQueryRunner.java
##########
@@ -71,19 +74,23 @@ public UnionQueryRunner(
         return new MergeSequence<>(
             query.getResultOrdering(),
             Sequences.simple(
-                Lists.transform(
-                    unionDataSource.getDataSources(),
-                    (Function<DataSource, Sequence<T>>) singleSource ->
-                        baseRunner.run(
-                            queryPlus.withQuery(
-                                Queries.withBaseDataSource(query, singleSource)
-                                       // assign the subqueryId. this will be used to validate that every query servers
-                                       // have responded per subquery in RetryQueryRunner
-                                       .withDefaultSubQueryId()
-                            ),
-                            responseContext
-                        )
-                )
+                IntStream.range(0, unionDataSource.getDataSources().size())
+                         .mapToObj(i -> new Pair<>(i + 1, unionDataSource.getDataSources().get(i)))
+                         .map(indexBaseDataSourcePair ->
+                                  baseRunner.run(
+                                      queryPlus.withQuery(Queries.withBaseDataSource(
+                                          query,
+                                          indexBaseDataSourcePair.rhs
+                                      ).withSubQueryId(
+                                          generateSubqueryId(
+                                              query.getSubQueryId(),
+                                              // getName() works since the datasource will be a TableDataSource
+                                              indexBaseDataSourcePair.rhs.getName(),
+                                              indexBaseDataSourcePair.lhs
+                                          )
+                                      ))

Review comment:
       ```suggestion
                                         )),
                                         responseContext
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on a change in pull request #11809:
URL: https://github.com/apache/druid/pull/11809#discussion_r741628171



##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();

Review comment:
       ```suggestion
       Map<QueryDataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
   ```

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));
+          ++siblingOrder;
+        }
+        queue.addAll(currentDataSource.getChildren());
+      }
+      ++level;
+    }
+    /*
+    Returns the datasource by populating all the subqueries with the id generated in the map above.
+    Implemented in a separate function since the methods on datasource and queries return a new datasource/query
+     */
+    return insertSubqueryIds(dataSource, queryDataSourceToSubqueryIds, parentQueryId, parentSqlQueryId);
+  }
+
+  /**
+   * To be used in conjunction with {@code generateSubqueryIds()} method. This does the actual task of populating the
+   * query's id, subQueryId and sqlQueryId
+   *
+   * @param dataSource                   The datasource to be populated with the subqueries
+   * @param queryDataSourceToSubqueryIds Map of the datasources to their level and sibling order
+   * @param parentQueryId                Parent query's id
+   * @param parentSqlQueryId             Parent query's sqlQueryId
+   * @return Populates the subqueries from the map
+   */
+  private DataSource insertSubqueryIds(
+      DataSource dataSource,
+      Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    if (queryDataSourceToSubqueryIds.containsKey(dataSource)) {
+      if (dataSource instanceof QueryDataSource) { // This should always be true, done for typecasting

Review comment:
       this will be redundant once you modify the signature of the map. 

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));

Review comment:
       I see. Thank you for explaining that. 

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));
+          ++siblingOrder;
+        }
+        queue.addAll(currentDataSource.getChildren());
+      }
+      ++level;
+    }
+    /*
+    Returns the datasource by populating all the subqueries with the id generated in the map above.
+    Implemented in a separate function since the methods on datasource and queries return a new datasource/query
+     */
+    return insertSubqueryIds(dataSource, queryDataSourceToSubqueryIds, parentQueryId, parentSqlQueryId);
+  }
+
+  /**
+   * To be used in conjunction with {@code generateSubqueryIds()} method. This does the actual task of populating the
+   * query's id, subQueryId and sqlQueryId
+   *
+   * @param dataSource                   The datasource to be populated with the subqueries
+   * @param queryDataSourceToSubqueryIds Map of the datasources to their level and sibling order
+   * @param parentQueryId                Parent query's id
+   * @param parentSqlQueryId             Parent query's sqlQueryId
+   * @return Populates the subqueries from the map
+   */
+  private DataSource insertSubqueryIds(
+      DataSource dataSource,
+      Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    if (queryDataSourceToSubqueryIds.containsKey(dataSource)) {
+      if (dataSource instanceof QueryDataSource) { // This should always be true, done for typecasting
+        QueryDataSource queryDataSource = (QueryDataSource) dataSource;
+        Pair<Integer, Integer> nestingInfo = queryDataSourceToSubqueryIds.get(dataSource);
+        String subQueryId = nestingInfo.lhs.toString() + "." + nestingInfo.rhs.toString();
+        Query<?> query = queryDataSource.getQuery();
+
+        if (StringUtils.isEmpty(query.getSubQueryId())) {
+          query = query.withSubQueryId(subQueryId);
+        }
+
+        if (StringUtils.isEmpty(query.getId()) && StringUtils.isNotEmpty(parentQueryId)) {
+          query = query.withId(parentQueryId);
+        }
+
+        if (StringUtils.isEmpty(query.getSqlQueryId()) && StringUtils.isNotEmpty(parentSqlQueryId)) {
+          query = query.withSqlQueryId(parentSqlQueryId);
+        }
+
+        queryDataSource = new QueryDataSource(query);
+        dataSource = queryDataSource;

Review comment:
       ```suggestion
           dataSource = new QueryDataSource(query);
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] LakshSingla commented on a change in pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
LakshSingla commented on a change in pull request #11809:
URL: https://github.com/apache/druid/pull/11809#discussion_r741615500



##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));

Review comment:
       The `Query#withSubqueryId()` method returns a new subquery with modified context, as does `Datasource#withChildren`. In order to actually modify the subquery id, we would have to go in a bottom's up manner (from the innermost datasource) and replace the child of the parent datasource, and then return this new parent datasource to be replaced in it's own parent datasource and so forth. I failed to come up with a way of doing that in a loop.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 closed pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 closed pull request #11809:
URL: https://github.com/apache/druid/pull/11809


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] LakshSingla commented on pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
LakshSingla commented on pull request #11809:
URL: https://github.com/apache/druid/pull/11809#issuecomment-961934475


   Not sure, however IT failure seems unrelated. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] FrankChen021 commented on pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on pull request #11809:
URL: https://github.com/apache/druid/pull/11809#issuecomment-965362150


   Hi @LakshSingla Thanks for your contribution. Please DO NOT force push since it makes it difficult for reviewers to see what you've changed in response to their reviews. See our [CONTRIBUTEING.md](https://github.com/apache/druid/blob/master/CONTRIBUTING.md)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 closed pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 closed pull request #11809:
URL: https://github.com/apache/druid/pull/11809


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 commented on pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on pull request #11809:
URL: https://github.com/apache/druid/pull/11809#issuecomment-966208821


   Thank you @LakshSingla for your contribution. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] LakshSingla closed pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
LakshSingla closed pull request #11809:
URL: https://github.com/apache/druid/pull/11809


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 closed pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 closed pull request #11809:
URL: https://github.com/apache/druid/pull/11809


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] LakshSingla commented on pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
LakshSingla commented on pull request #11809:
URL: https://github.com/apache/druid/pull/11809#issuecomment-953768873


   The format of the Subquery ID has been changed in the later commits. Subquery ID is divided into 3 parts separated by `-`. (not all parts maybe present for a given subquery).
   1. First part indicates the sequence in which the subquery is run (in the collection of subqueries generated by the master query).
   2. The second part indicates the nesting level of that subquery datasource in the structure of original datasource. The nesting structure represents a hierarchy delimited by `.`.
   3. The third part is present if the subqueries are generated as a response to a union query, and depicts the original datasource and it's sequence number in the union subqueries.
   
   Example queries: 
   ```
   GroupByQuery{                                           
     dataSource='JoinDataSource{                         
       left=UnionDataSource{dataSources=[                  
         foo,                                            
         bar]},                                      
       right=GroupByQuery{                     
         dataSource='UnionDataSource{dataSources=[       
           foo,                                            
           bar]}                                         
     }
   }     
   ```
   This will generate 4 subqueries. The one generated due to inlining of inner `GroupByQuery` would have sequence number as 1 (first of the subqueries which is run), the nesting level as `1.2` (nesting level of the subquery inside the parent datasource), and the third part a foo.1 and bar.2 respectively. Therefore the subqueries thus generated would have ids: `1-1.2-foo.1` and `1-1.2-bar.2`
   The original query run would have the subquery ids of `foo.1` and `bar.2` (denoting the union datasources) since they don't contain the first and the second part respectively. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on a change in pull request #11809:
URL: https://github.com/apache/druid/pull/11809#discussion_r740761353



##########
File path: processing/src/main/java/org/apache/druid/query/UnionQueryRunner.java
##########
@@ -93,4 +102,25 @@ public UnionQueryRunner(
     }
   }
 
+  /**
+   * Appends and returns the name and the position of the individual datasource in the union with the parent query id
+   * if preseent
+   *
+   * @param parentSubqueryId The subquery Id of the parent query which is generating this subquery
+   * @param dataSourceName   Name of the datasource for which the UnionRunner is running
+   * @param dataSourceIndex  Position of the datasource for which the UnionRunner is running
+   * @return Subquery Id which needs to be populated
+   */
+  private String generateSubqueryId(String parentSubqueryId, String dataSourceName, int dataSourceIndex)
+  {
+    final String DELIMITER = "-";
+    List<String> arr = new ArrayList<>();

Review comment:
       do we need an array? 

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -147,15 +151,23 @@ public ClientQuerySegmentWalker(
 
     // transform TableDataSource to GlobalTableDataSource when eligible
     // before further transformation to potentially inline
-    final DataSource freeTradeDataSource = globalizeIfPossible(query.getDataSource());
+
+    // Populate the subquery ids of the subquery id present in the main query
+    Query<T> newQuery = query.withDataSource(generateSubqueryIds(query.getDataSource(), "1"));

Review comment:
       ```suggestion
       Query<T> queryWithSubqueryIds = query.withDataSource(generateSubqueryIds(query.getDataSource(), "1"));
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] LakshSingla commented on a change in pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
LakshSingla commented on a change in pull request #11809:
URL: https://github.com/apache/druid/pull/11809#discussion_r741574999



##########
File path: processing/src/main/java/org/apache/druid/query/UnionQueryRunner.java
##########
@@ -93,4 +102,25 @@ public UnionQueryRunner(
     }
   }
 
+  /**
+   * Appends and returns the name and the position of the individual datasource in the union with the parent query id
+   * if preseent
+   *
+   * @param parentSubqueryId The subquery Id of the parent query which is generating this subquery
+   * @param dataSourceName   Name of the datasource for which the UnionRunner is running
+   * @param dataSourceIndex  Position of the datasource for which the UnionRunner is running
+   * @return Subquery Id which needs to be populated
+   */
+  private String generateSubqueryId(String parentSubqueryId, String dataSourceName, int dataSourceIndex)
+  {
+    final String DELIMITER = "-";
+    List<String> arr = new ArrayList<>();

Review comment:
       Updated the code, was leftover from my previous implementation.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 closed pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 closed pull request #11809:
URL: https://github.com/apache/druid/pull/11809


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] LakshSingla commented on pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
LakshSingla commented on pull request #11809:
URL: https://github.com/apache/druid/pull/11809#issuecomment-961934475


   Not sure, however IT failure seems unrelated. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on a change in pull request #11809:
URL: https://github.com/apache/druid/pull/11809#discussion_r741613829



##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));

Review comment:
       It is not clear to me why we are not calling `insertSubQueryId` here itself. That is we have the level information already so we can populate the ids for the query corresponding to this QueryDataSource here itself instead of saving it inside a map and doing it later. am I missing something? 

##########
File path: processing/src/main/java/org/apache/druid/query/UnionQueryRunner.java
##########
@@ -71,19 +74,23 @@ public UnionQueryRunner(
         return new MergeSequence<>(
             query.getResultOrdering(),
             Sequences.simple(
-                Lists.transform(
-                    unionDataSource.getDataSources(),
-                    (Function<DataSource, Sequence<T>>) singleSource ->
-                        baseRunner.run(
-                            queryPlus.withQuery(
-                                Queries.withBaseDataSource(query, singleSource)
-                                       // assign the subqueryId. this will be used to validate that every query servers
-                                       // have responded per subquery in RetryQueryRunner
-                                       .withDefaultSubQueryId()
-                            ),
-                            responseContext
-                        )
-                )
+                IntStream.range(0, unionDataSource.getDataSources().size())
+                         .mapToObj(i -> new Pair<>(i + 1, unionDataSource.getDataSources().get(i)))
+                         .map(indexBaseDataSourcePair ->
+                                  baseRunner.run(
+                                      queryPlus.withQuery(Queries.withBaseDataSource(
+                                          query,
+                                          indexBaseDataSourcePair.rhs
+                                      ).withSubQueryId(
+                                          generateSubqueryId(
+                                              query.getSubQueryId(),
+                                              // toString() works since the datasource will be a TableDataSource
+                                              indexBaseDataSourcePair.rhs.toString(),

Review comment:
       maybe you can call `getName()` instead of `toString()`? 

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();

Review comment:
       ```suggestion
       Map<QueryDataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
   ```

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));
+          ++siblingOrder;
+        }
+        queue.addAll(currentDataSource.getChildren());
+      }
+      ++level;
+    }
+    /*
+    Returns the datasource by populating all the subqueries with the id generated in the map above.
+    Implemented in a separate function since the methods on datasource and queries return a new datasource/query
+     */
+    return insertSubqueryIds(dataSource, queryDataSourceToSubqueryIds, parentQueryId, parentSqlQueryId);
+  }
+
+  /**
+   * To be used in conjunction with {@code generateSubqueryIds()} method. This does the actual task of populating the
+   * query's id, subQueryId and sqlQueryId
+   *
+   * @param dataSource                   The datasource to be populated with the subqueries
+   * @param queryDataSourceToSubqueryIds Map of the datasources to their level and sibling order
+   * @param parentQueryId                Parent query's id
+   * @param parentSqlQueryId             Parent query's sqlQueryId
+   * @return Populates the subqueries from the map
+   */
+  private DataSource insertSubqueryIds(
+      DataSource dataSource,
+      Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    if (queryDataSourceToSubqueryIds.containsKey(dataSource)) {
+      if (dataSource instanceof QueryDataSource) { // This should always be true, done for typecasting

Review comment:
       this will be redundant once you modify the signature of the map. 

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));

Review comment:
       I see. Thank you for explaining that. 

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));
+          ++siblingOrder;
+        }
+        queue.addAll(currentDataSource.getChildren());
+      }
+      ++level;
+    }
+    /*
+    Returns the datasource by populating all the subqueries with the id generated in the map above.
+    Implemented in a separate function since the methods on datasource and queries return a new datasource/query
+     */
+    return insertSubqueryIds(dataSource, queryDataSourceToSubqueryIds, parentQueryId, parentSqlQueryId);
+  }
+
+  /**
+   * To be used in conjunction with {@code generateSubqueryIds()} method. This does the actual task of populating the
+   * query's id, subQueryId and sqlQueryId
+   *
+   * @param dataSource                   The datasource to be populated with the subqueries
+   * @param queryDataSourceToSubqueryIds Map of the datasources to their level and sibling order
+   * @param parentQueryId                Parent query's id
+   * @param parentSqlQueryId             Parent query's sqlQueryId
+   * @return Populates the subqueries from the map
+   */
+  private DataSource insertSubqueryIds(
+      DataSource dataSource,
+      Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    if (queryDataSourceToSubqueryIds.containsKey(dataSource)) {
+      if (dataSource instanceof QueryDataSource) { // This should always be true, done for typecasting
+        QueryDataSource queryDataSource = (QueryDataSource) dataSource;
+        Pair<Integer, Integer> nestingInfo = queryDataSourceToSubqueryIds.get(dataSource);
+        String subQueryId = nestingInfo.lhs.toString() + "." + nestingInfo.rhs.toString();
+        Query<?> query = queryDataSource.getQuery();
+
+        if (StringUtils.isEmpty(query.getSubQueryId())) {
+          query = query.withSubQueryId(subQueryId);
+        }
+
+        if (StringUtils.isEmpty(query.getId()) && StringUtils.isNotEmpty(parentQueryId)) {
+          query = query.withId(parentQueryId);
+        }
+
+        if (StringUtils.isEmpty(query.getSqlQueryId()) && StringUtils.isNotEmpty(parentSqlQueryId)) {
+          query = query.withSqlQueryId(parentSqlQueryId);
+        }
+
+        queryDataSource = new QueryDataSource(query);
+        dataSource = queryDataSource;

Review comment:
       ```suggestion
           dataSource = new QueryDataSource(query);
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] LakshSingla commented on a change in pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
LakshSingla commented on a change in pull request #11809:
URL: https://github.com/apache/druid/pull/11809#discussion_r741615500



##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));

Review comment:
       The `Query#withSubqueryId()` method returns a new subquery with modified context, as does `Datasource#withChildren`. In order to actually modify the subquery id, we would have to go in a bottom's up manner (from the innermost datasource) and replace the child of the parent datasource, and then return this new parent datasource to be replaced in it's own parent datasource and so forth. I failed to come up with a way of doing that in a loop.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 merged pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 merged pull request #11809:
URL: https://github.com/apache/druid/pull/11809


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on a change in pull request #11809:
URL: https://github.com/apache/druid/pull/11809#discussion_r741613829



##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));

Review comment:
       It is not clear to me why we are not calling `insertSubQueryId` here itself. That is we have the level information already so we can populate the ids for the query corresponding to this QueryDataSource here itself instead of saving it inside a map and doing it later. am I missing something? 

##########
File path: processing/src/main/java/org/apache/druid/query/UnionQueryRunner.java
##########
@@ -71,19 +74,23 @@ public UnionQueryRunner(
         return new MergeSequence<>(
             query.getResultOrdering(),
             Sequences.simple(
-                Lists.transform(
-                    unionDataSource.getDataSources(),
-                    (Function<DataSource, Sequence<T>>) singleSource ->
-                        baseRunner.run(
-                            queryPlus.withQuery(
-                                Queries.withBaseDataSource(query, singleSource)
-                                       // assign the subqueryId. this will be used to validate that every query servers
-                                       // have responded per subquery in RetryQueryRunner
-                                       .withDefaultSubQueryId()
-                            ),
-                            responseContext
-                        )
-                )
+                IntStream.range(0, unionDataSource.getDataSources().size())
+                         .mapToObj(i -> new Pair<>(i + 1, unionDataSource.getDataSources().get(i)))
+                         .map(indexBaseDataSourcePair ->
+                                  baseRunner.run(
+                                      queryPlus.withQuery(Queries.withBaseDataSource(
+                                          query,
+                                          indexBaseDataSourcePair.rhs
+                                      ).withSubQueryId(
+                                          generateSubqueryId(
+                                              query.getSubQueryId(),
+                                              // toString() works since the datasource will be a TableDataSource
+                                              indexBaseDataSourcePair.rhs.toString(),

Review comment:
       maybe you can call `getName()` instead of `toString()`? 

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();

Review comment:
       ```suggestion
       Map<QueryDataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
   ```

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));
+          ++siblingOrder;
+        }
+        queue.addAll(currentDataSource.getChildren());
+      }
+      ++level;
+    }
+    /*
+    Returns the datasource by populating all the subqueries with the id generated in the map above.
+    Implemented in a separate function since the methods on datasource and queries return a new datasource/query
+     */
+    return insertSubqueryIds(dataSource, queryDataSourceToSubqueryIds, parentQueryId, parentSqlQueryId);
+  }
+
+  /**
+   * To be used in conjunction with {@code generateSubqueryIds()} method. This does the actual task of populating the
+   * query's id, subQueryId and sqlQueryId
+   *
+   * @param dataSource                   The datasource to be populated with the subqueries
+   * @param queryDataSourceToSubqueryIds Map of the datasources to their level and sibling order
+   * @param parentQueryId                Parent query's id
+   * @param parentSqlQueryId             Parent query's sqlQueryId
+   * @return Populates the subqueries from the map
+   */
+  private DataSource insertSubqueryIds(
+      DataSource dataSource,
+      Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    if (queryDataSourceToSubqueryIds.containsKey(dataSource)) {
+      if (dataSource instanceof QueryDataSource) { // This should always be true, done for typecasting

Review comment:
       this will be redundant once you modify the signature of the map. 

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));

Review comment:
       I see. Thank you for explaining that. 

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));
+          ++siblingOrder;
+        }
+        queue.addAll(currentDataSource.getChildren());
+      }
+      ++level;
+    }
+    /*
+    Returns the datasource by populating all the subqueries with the id generated in the map above.
+    Implemented in a separate function since the methods on datasource and queries return a new datasource/query
+     */
+    return insertSubqueryIds(dataSource, queryDataSourceToSubqueryIds, parentQueryId, parentSqlQueryId);
+  }
+
+  /**
+   * To be used in conjunction with {@code generateSubqueryIds()} method. This does the actual task of populating the
+   * query's id, subQueryId and sqlQueryId
+   *
+   * @param dataSource                   The datasource to be populated with the subqueries
+   * @param queryDataSourceToSubqueryIds Map of the datasources to their level and sibling order
+   * @param parentQueryId                Parent query's id
+   * @param parentSqlQueryId             Parent query's sqlQueryId
+   * @return Populates the subqueries from the map
+   */
+  private DataSource insertSubqueryIds(
+      DataSource dataSource,
+      Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    if (queryDataSourceToSubqueryIds.containsKey(dataSource)) {
+      if (dataSource instanceof QueryDataSource) { // This should always be true, done for typecasting
+        QueryDataSource queryDataSource = (QueryDataSource) dataSource;
+        Pair<Integer, Integer> nestingInfo = queryDataSourceToSubqueryIds.get(dataSource);
+        String subQueryId = nestingInfo.lhs.toString() + "." + nestingInfo.rhs.toString();
+        Query<?> query = queryDataSource.getQuery();
+
+        if (StringUtils.isEmpty(query.getSubQueryId())) {
+          query = query.withSubQueryId(subQueryId);
+        }
+
+        if (StringUtils.isEmpty(query.getId()) && StringUtils.isNotEmpty(parentQueryId)) {
+          query = query.withId(parentQueryId);
+        }
+
+        if (StringUtils.isEmpty(query.getSqlQueryId()) && StringUtils.isNotEmpty(parentSqlQueryId)) {
+          query = query.withSqlQueryId(parentSqlQueryId);
+        }
+
+        queryDataSource = new QueryDataSource(query);
+        dataSource = queryDataSource;

Review comment:
       ```suggestion
           dataSource = new QueryDataSource(query);
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] LakshSingla commented on a change in pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
LakshSingla commented on a change in pull request #11809:
URL: https://github.com/apache/druid/pull/11809#discussion_r741574999



##########
File path: processing/src/main/java/org/apache/druid/query/UnionQueryRunner.java
##########
@@ -93,4 +102,25 @@ public UnionQueryRunner(
     }
   }
 
+  /**
+   * Appends and returns the name and the position of the individual datasource in the union with the parent query id
+   * if preseent
+   *
+   * @param parentSubqueryId The subquery Id of the parent query which is generating this subquery
+   * @param dataSourceName   Name of the datasource for which the UnionRunner is running
+   * @param dataSourceIndex  Position of the datasource for which the UnionRunner is running
+   * @return Subquery Id which needs to be populated
+   */
+  private String generateSubqueryId(String parentSubqueryId, String dataSourceName, int dataSourceIndex)
+  {
+    final String DELIMITER = "-";
+    List<String> arr = new ArrayList<>();

Review comment:
       Updated the code, was leftover from my previous implementation.

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));

Review comment:
       The `Query#withSubqueryId()` method returns a new subquery with modified context, as does `Datasource#withChildren`. In order to actually modify the subquery id, we would have to go in a bottom's up manner (from the innermost datasource) and replace the child of the parent datasource, and then return this new parent datasource to be replaced in it's own parent datasource and so forth. I failed to come up with a way of doing that in a loop.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on a change in pull request #11809:
URL: https://github.com/apache/druid/pull/11809#discussion_r741613829



##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));

Review comment:
       It is not clear to me why we are not calling `insertSubQueryId` here itself. That is we have the level information already so we can populate the ids for the query corresponding to this QueryDataSource here itself instead of saving it inside a map and doing it later. am I missing something? 

##########
File path: processing/src/main/java/org/apache/druid/query/UnionQueryRunner.java
##########
@@ -71,19 +74,23 @@ public UnionQueryRunner(
         return new MergeSequence<>(
             query.getResultOrdering(),
             Sequences.simple(
-                Lists.transform(
-                    unionDataSource.getDataSources(),
-                    (Function<DataSource, Sequence<T>>) singleSource ->
-                        baseRunner.run(
-                            queryPlus.withQuery(
-                                Queries.withBaseDataSource(query, singleSource)
-                                       // assign the subqueryId. this will be used to validate that every query servers
-                                       // have responded per subquery in RetryQueryRunner
-                                       .withDefaultSubQueryId()
-                            ),
-                            responseContext
-                        )
-                )
+                IntStream.range(0, unionDataSource.getDataSources().size())
+                         .mapToObj(i -> new Pair<>(i + 1, unionDataSource.getDataSources().get(i)))
+                         .map(indexBaseDataSourcePair ->
+                                  baseRunner.run(
+                                      queryPlus.withQuery(Queries.withBaseDataSource(
+                                          query,
+                                          indexBaseDataSourcePair.rhs
+                                      ).withSubQueryId(
+                                          generateSubqueryId(
+                                              query.getSubQueryId(),
+                                              // toString() works since the datasource will be a TableDataSource
+                                              indexBaseDataSourcePair.rhs.toString(),

Review comment:
       maybe you can call `getName()` instead of `toString()`? 

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();

Review comment:
       ```suggestion
       Map<QueryDataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
   ```

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));
+          ++siblingOrder;
+        }
+        queue.addAll(currentDataSource.getChildren());
+      }
+      ++level;
+    }
+    /*
+    Returns the datasource by populating all the subqueries with the id generated in the map above.
+    Implemented in a separate function since the methods on datasource and queries return a new datasource/query
+     */
+    return insertSubqueryIds(dataSource, queryDataSourceToSubqueryIds, parentQueryId, parentSqlQueryId);
+  }
+
+  /**
+   * To be used in conjunction with {@code generateSubqueryIds()} method. This does the actual task of populating the
+   * query's id, subQueryId and sqlQueryId
+   *
+   * @param dataSource                   The datasource to be populated with the subqueries
+   * @param queryDataSourceToSubqueryIds Map of the datasources to their level and sibling order
+   * @param parentQueryId                Parent query's id
+   * @param parentSqlQueryId             Parent query's sqlQueryId
+   * @return Populates the subqueries from the map
+   */
+  private DataSource insertSubqueryIds(
+      DataSource dataSource,
+      Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    if (queryDataSourceToSubqueryIds.containsKey(dataSource)) {
+      if (dataSource instanceof QueryDataSource) { // This should always be true, done for typecasting

Review comment:
       this will be redundant once you modify the signature of the map. 

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));

Review comment:
       I see. Thank you for explaining that. 

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));
+          ++siblingOrder;
+        }
+        queue.addAll(currentDataSource.getChildren());
+      }
+      ++level;
+    }
+    /*
+    Returns the datasource by populating all the subqueries with the id generated in the map above.
+    Implemented in a separate function since the methods on datasource and queries return a new datasource/query
+     */
+    return insertSubqueryIds(dataSource, queryDataSourceToSubqueryIds, parentQueryId, parentSqlQueryId);
+  }
+
+  /**
+   * To be used in conjunction with {@code generateSubqueryIds()} method. This does the actual task of populating the
+   * query's id, subQueryId and sqlQueryId
+   *
+   * @param dataSource                   The datasource to be populated with the subqueries
+   * @param queryDataSourceToSubqueryIds Map of the datasources to their level and sibling order
+   * @param parentQueryId                Parent query's id
+   * @param parentSqlQueryId             Parent query's sqlQueryId
+   * @return Populates the subqueries from the map
+   */
+  private DataSource insertSubqueryIds(
+      DataSource dataSource,
+      Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    if (queryDataSourceToSubqueryIds.containsKey(dataSource)) {
+      if (dataSource instanceof QueryDataSource) { // This should always be true, done for typecasting
+        QueryDataSource queryDataSource = (QueryDataSource) dataSource;
+        Pair<Integer, Integer> nestingInfo = queryDataSourceToSubqueryIds.get(dataSource);
+        String subQueryId = nestingInfo.lhs.toString() + "." + nestingInfo.rhs.toString();
+        Query<?> query = queryDataSource.getQuery();
+
+        if (StringUtils.isEmpty(query.getSubQueryId())) {
+          query = query.withSubQueryId(subQueryId);
+        }
+
+        if (StringUtils.isEmpty(query.getId()) && StringUtils.isNotEmpty(parentQueryId)) {
+          query = query.withId(parentQueryId);
+        }
+
+        if (StringUtils.isEmpty(query.getSqlQueryId()) && StringUtils.isNotEmpty(parentSqlQueryId)) {
+          query = query.withSqlQueryId(parentSqlQueryId);
+        }
+
+        queryDataSource = new QueryDataSource(query);
+        dataSource = queryDataSource;

Review comment:
       ```suggestion
           dataSource = new QueryDataSource(query);
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 closed pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 closed pull request #11809:
URL: https://github.com/apache/druid/pull/11809


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on a change in pull request #11809:
URL: https://github.com/apache/druid/pull/11809#discussion_r741613829



##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -431,6 +448,101 @@ private DataSource inlineIfNecessary(
         );
   }
 
+  /**
+   * This method returns the datasource by populating all the {@link QueryDataSource} with correct nesting level and
+   * sibling order of all the subqueries that are present.
+   * It also plumbs parent query's id and sql id in case the subqueries don't have it set by default
+   *
+   * @param dataSource       Datasource whose subqueries need to be populated
+   * @param parentQueryId    Parent Query's ID, can be null if do not need to update this in the subqueries
+   * @param parentSqlQueryId Parent Query's SQL Query ID, can be null if do not need to update this in the subqueries
+   * @return DataSource populated with the subqueries
+   */
+  private DataSource generateSubqueryIds(
+      DataSource dataSource,
+      @Nullable final String parentQueryId,
+      @Nullable final String parentSqlQueryId
+  )
+  {
+    Queue<DataSource> queue = new LinkedList<>();
+    queue.add(dataSource);
+
+    /*
+    Performs BFS on the datasource tree to find the nesting level, and the sibling order of the query datasource
+     */
+    Map<DataSource, Pair<Integer, Integer>> queryDataSourceToSubqueryIds = new HashMap<>();
+    int level = 1;
+    while (!queue.isEmpty()) {
+      int size = queue.size();
+      int siblingOrder = 1;
+      for (int i = 0; i < size; ++i) {
+        DataSource currentDataSource = queue.poll();
+        if (currentDataSource instanceof QueryDataSource) {
+          queryDataSourceToSubqueryIds.put(currentDataSource, new Pair<>(level, siblingOrder));

Review comment:
       It is not clear to me why we are not calling `insertSubQueryId` here itself. That is we have the level information already so we can populate the ids for the query corresponding to this QueryDataSource here itself instead of saving it inside a map and doing it later. am I missing something? 

##########
File path: processing/src/main/java/org/apache/druid/query/UnionQueryRunner.java
##########
@@ -71,19 +74,23 @@ public UnionQueryRunner(
         return new MergeSequence<>(
             query.getResultOrdering(),
             Sequences.simple(
-                Lists.transform(
-                    unionDataSource.getDataSources(),
-                    (Function<DataSource, Sequence<T>>) singleSource ->
-                        baseRunner.run(
-                            queryPlus.withQuery(
-                                Queries.withBaseDataSource(query, singleSource)
-                                       // assign the subqueryId. this will be used to validate that every query servers
-                                       // have responded per subquery in RetryQueryRunner
-                                       .withDefaultSubQueryId()
-                            ),
-                            responseContext
-                        )
-                )
+                IntStream.range(0, unionDataSource.getDataSources().size())
+                         .mapToObj(i -> new Pair<>(i + 1, unionDataSource.getDataSources().get(i)))
+                         .map(indexBaseDataSourcePair ->
+                                  baseRunner.run(
+                                      queryPlus.withQuery(Queries.withBaseDataSource(
+                                          query,
+                                          indexBaseDataSourcePair.rhs
+                                      ).withSubQueryId(
+                                          generateSubqueryId(
+                                              query.getSubQueryId(),
+                                              // toString() works since the datasource will be a TableDataSource
+                                              indexBaseDataSourcePair.rhs.toString(),

Review comment:
       maybe you can call `getName()` instead of `toString()`? 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on a change in pull request #11809:
URL: https://github.com/apache/druid/pull/11809#discussion_r740761353



##########
File path: processing/src/main/java/org/apache/druid/query/UnionQueryRunner.java
##########
@@ -93,4 +102,25 @@ public UnionQueryRunner(
     }
   }
 
+  /**
+   * Appends and returns the name and the position of the individual datasource in the union with the parent query id
+   * if preseent
+   *
+   * @param parentSubqueryId The subquery Id of the parent query which is generating this subquery
+   * @param dataSourceName   Name of the datasource for which the UnionRunner is running
+   * @param dataSourceIndex  Position of the datasource for which the UnionRunner is running
+   * @return Subquery Id which needs to be populated
+   */
+  private String generateSubqueryId(String parentSubqueryId, String dataSourceName, int dataSourceIndex)
+  {
+    final String DELIMITER = "-";
+    List<String> arr = new ArrayList<>();

Review comment:
       do we need an array? 

##########
File path: server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
##########
@@ -147,15 +151,23 @@ public ClientQuerySegmentWalker(
 
     // transform TableDataSource to GlobalTableDataSource when eligible
     // before further transformation to potentially inline
-    final DataSource freeTradeDataSource = globalizeIfPossible(query.getDataSource());
+
+    // Populate the subquery ids of the subquery id present in the main query
+    Query<T> newQuery = query.withDataSource(generateSubqueryIds(query.getDataSource(), "1"));

Review comment:
       ```suggestion
       Query<T> queryWithSubqueryIds = query.withDataSource(generateSubqueryIds(query.getDataSource(), "1"));
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 closed pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 closed pull request #11809:
URL: https://github.com/apache/druid/pull/11809


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] LakshSingla commented on pull request #11809: Make subquery IDs more comprehensive

Posted by GitBox <gi...@apache.org>.
LakshSingla commented on pull request #11809:
URL: https://github.com/apache/druid/pull/11809#issuecomment-961934475


   Not sure, however IT failure seems unrelated. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org