[GitHub] drill pull request #1116: DRILL-6140: Correctly list Operators in a Query Pr...
Github user asfgit closed the pull request at: https://github.com/apache/drill/pull/1116 ---
[GitHub] drill pull request #1116: DRILL-6140: Correctly list Operators in a Query Pr...
Github user kkhatua commented on a diff in the pull request: https://github.com/apache/drill/pull/1116#discussion_r167649909 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileWrapper.java --- @@ -322,4 +330,16 @@ public String getOperatorsJSON() { public boolean isOnlyImpersonationEnabled() { return onlyImpersonationEnabled; } + + //Generates operator names inferred from physical plan + private void generateOpMap(String plan) { +this.physicalOperatorMap = new HashMap(); +String[] operatorLine = plan.split("\\n"); +for (String line : operatorLine) { + String[] lineToken = line.split("\\s+", 3); + String operatorPath = lineToken[0].trim().replaceFirst("-", "-xx-"); //Required format for lookup --- End diff -- Added the following as the example ``` 01-03 Flatten(flattenField=[$1]) : rowType = RecordType(ANY rfsSpecCode, ... ``` ---
[GitHub] drill pull request #1116: DRILL-6140: Correctly list Operators in a Query Pr...
Github user kkhatua commented on a diff in the pull request: https://github.com/apache/drill/pull/1116#discussion_r167649725 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java --- @@ -45,12 +45,25 @@ private final String operatorName; private final int size; - public OperatorWrapper(int major, List, String>> opsAndHostsList) { + public OperatorWrapper(int major, List, String>> opsAndHostsList, Map phyOperMap) { Preconditions.checkArgument(opsAndHostsList.size() > 0); this.major = major; firstProfile = opsAndHostsList.get(0).getLeft().getLeft(); operatorType = CoreOperatorType.valueOf(firstProfile.getOperatorType()); -operatorName = operatorType == null ? UNKNOWN_OPERATOR : operatorType.toString(); +//Update Name from Physical Map +String path = new OperatorPathBuilder().setMajor(major).setOperator(firstProfile).build(); +//Use Plan Extracted Operator Names if available +String extractedOpName = phyOperMap.get(path); +String inferredOpName = operatorType == null ? UNKNOWN_OPERATOR : operatorType.toString(); +//Revert to inferred names for exceptional cases +if (extractedOpName == null || inferredOpName.contains(extractedOpName) +|| inferredOpName.endsWith("_RECEIVER") || inferredOpName.endsWith("_RECEIVER")) { --- End diff -- Fixed it. Refactored to checking for *_EXCHANGE ---
[GitHub] drill pull request #1116: DRILL-6140: Correctly list Operators in a Query Pr...
Github user kkhatua commented on a diff in the pull request: https://github.com/apache/drill/pull/1116#discussion_r167649582 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java --- @@ -45,12 +45,25 @@ private final String operatorName; private final int size; - public OperatorWrapper(int major, List, String>> opsAndHostsList) { + public OperatorWrapper(int major, List, String>> opsAndHostsList, Map phyOperMap) { Preconditions.checkArgument(opsAndHostsList.size() > 0); this.major = major; firstProfile = opsAndHostsList.get(0).getLeft().getLeft(); operatorType = CoreOperatorType.valueOf(firstProfile.getOperatorType()); -operatorName = operatorType == null ? UNKNOWN_OPERATOR : operatorType.toString(); +//Update Name from Physical Map +String path = new OperatorPathBuilder().setMajor(major).setOperator(firstProfile).build(); +//Use Plan Extracted Operator Names if available +String extractedOpName = phyOperMap.get(path); +String inferredOpName = operatorType == null ? UNKNOWN_OPERATOR : operatorType.toString(); +//Revert to inferred names for exceptional cases --- End diff -- Added all the exceptions in comments ---
[GitHub] drill pull request #1116: DRILL-6140: Correctly list Operators in a Query Pr...
Github user amansinha100 commented on a diff in the pull request: https://github.com/apache/drill/pull/1116#discussion_r167286191 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java --- @@ -45,12 +45,25 @@ private final String operatorName; private final int size; - public OperatorWrapper(int major, List, String>> opsAndHostsList) { + public OperatorWrapper(int major, List, String>> opsAndHostsList, Map phyOperMap) { Preconditions.checkArgument(opsAndHostsList.size() > 0); this.major = major; firstProfile = opsAndHostsList.get(0).getLeft().getLeft(); operatorType = CoreOperatorType.valueOf(firstProfile.getOperatorType()); -operatorName = operatorType == null ? UNKNOWN_OPERATOR : operatorType.toString(); +//Update Name from Physical Map +String path = new OperatorPathBuilder().setMajor(major).setOperator(firstProfile).build(); +//Use Plan Extracted Operator Names if available +String extractedOpName = phyOperMap.get(path); +String inferredOpName = operatorType == null ? UNKNOWN_OPERATOR : operatorType.toString(); +//Revert to inferred names for exceptional cases +if (extractedOpName == null || inferredOpName.contains(extractedOpName) +|| inferredOpName.endsWith("_RECEIVER") || inferredOpName.endsWith("_RECEIVER")) { --- End diff -- There is an extra check for "_RECEIVER" ---
[GitHub] drill pull request #1116: DRILL-6140: Correctly list Operators in a Query Pr...
Github user amansinha100 commented on a diff in the pull request: https://github.com/apache/drill/pull/1116#discussion_r167288758 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/OperatorWrapper.java --- @@ -45,12 +45,25 @@ private final String operatorName; private final int size; - public OperatorWrapper(int major, List, String>> opsAndHostsList) { + public OperatorWrapper(int major, List, String>> opsAndHostsList, Map phyOperMap) { Preconditions.checkArgument(opsAndHostsList.size() > 0); this.major = major; firstProfile = opsAndHostsList.get(0).getLeft().getLeft(); operatorType = CoreOperatorType.valueOf(firstProfile.getOperatorType()); -operatorName = operatorType == null ? UNKNOWN_OPERATOR : operatorType.toString(); +//Update Name from Physical Map +String path = new OperatorPathBuilder().setMajor(major).setOperator(firstProfile).build(); +//Use Plan Extracted Operator Names if available +String extractedOpName = phyOperMap.get(path); +String inferredOpName = operatorType == null ? UNKNOWN_OPERATOR : operatorType.toString(); +//Revert to inferred names for exceptional cases --- End diff -- Can you clarify 'exception cases' .. there are 2 examples below but it is not immediately clear which operators would fall into this pattern. I assume the main mismatch is that the physical plan has EXCHANGE operators whereas the profile has SENDER and RECEIVER. ---
[GitHub] drill pull request #1116: DRILL-6140: Correctly list Operators in a Query Pr...
Github user amansinha100 commented on a diff in the pull request: https://github.com/apache/drill/pull/1116#discussion_r167269862 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileWrapper.java --- @@ -322,4 +330,16 @@ public String getOperatorsJSON() { public boolean isOnlyImpersonationEnabled() { return onlyImpersonationEnabled; } + + //Generates operator names inferred from physical plan + private void generateOpMap(String plan) { +this.physicalOperatorMap = new HashMap(); +String[] operatorLine = plan.split("\\n"); +for (String line : operatorLine) { + String[] lineToken = line.split("\\s+", 3); + String operatorPath = lineToken[0].trim().replaceFirst("-", "-xx-"); //Required format for lookup --- End diff -- Can you add (as a comment) the sample text from the physical plan output that this is parsing. ---