[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12543 ) Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning IMPALA-7694 added a field in the middle of the Metrics.TUnit enum, which broke backwards compatibility with profiles that had been written by older versions of Impala. This change fixes the ordering by moving the field to the end of the enum. Additionally, it adds a warning to the top of all Thrift files that are part of the binary profile format, and an note of caution to the main definition in RuntimeProfile.thrift. This change also fixes the order of all enums in our Thrift files to make errors like this less likely in the future. Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Reviewed-on: http://gerrit.cloudera.org:8080/12543 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M common/thrift/BackendGflags.thrift M common/thrift/CatalogObjects.thrift M common/thrift/DataSinks.thrift M common/thrift/ExecStats.thrift M common/thrift/Exprs.thrift M common/thrift/ExternalDataSource.thrift M common/thrift/Frontend.thrift M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M common/thrift/JniCatalog.thrift M common/thrift/LineageGraph.thrift M common/thrift/Logging.thrift M common/thrift/Metrics.thrift M common/thrift/Partitions.thrift M common/thrift/PlanNodes.thrift M common/thrift/RuntimeProfile.thrift M common/thrift/StatestoreService.thrift M common/thrift/Status.thrift M common/thrift/Types.thrift M common/thrift/beeswax.thrift 20 files changed, 482 insertions(+), 444 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 8 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12543 ) Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 7 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 22 Feb 2019 04:01:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12543 ) Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 7 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Feb 2019 23:57:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12543 ) Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2207/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 6 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 22 Feb 2019 00:24:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/12543 ) Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 6 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Feb 2019 23:51:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12543 ) Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3816/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 7 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Feb 2019 23:57:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12543 ) Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/12543/5/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/12543/5/common/thrift/ImpalaService.thrift@61 PS5, Line 61: // > 1: executes on at most that many nodes at any point in time (ie there can be : // more nodes than numNodes with plan fragments for this query but at most : // numNodes would be active at any point in time) : // Constants (NUM_NODES_ALL NUM_NODES_ALL_RACKS) are defined in JavaConstants.thrift. > Nit: This change removes a bunch of commas in various comments. Maybe some Thanks for catching this one, fixed it. http://gerrit.cloudera.org:8080/#/c/12543/5/common/thrift/PlanNodes.thrift File common/thrift/PlanNodes.thrift: http://gerrit.cloudera.org:8080/#/c/12543/5/common/thrift/PlanNodes.thrift@75 PS5, Line 75: // A floating point number in range [0.0 1.0] that gives the probability of denying > Same comma missing thing here and other comments in this file. Done -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 5 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Feb 2019 23:41:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Hello Philip Zeyliger, Tim Armstrong, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12543 to look at the new patch set (#6). Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning IMPALA-7694 added a field in the middle of the Metrics.TUnit enum, which broke backwards compatibility with profiles that had been written by older versions of Impala. This change fixes the ordering by moving the field to the end of the enum. Additionally, it adds a warning to the top of all Thrift files that are part of the binary profile format, and an note of caution to the main definition in RuntimeProfile.thrift. This change also fixes the order of all enums in our Thrift files to make errors like this less likely in the future. Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 --- M common/thrift/BackendGflags.thrift M common/thrift/CatalogObjects.thrift M common/thrift/DataSinks.thrift M common/thrift/ExecStats.thrift M common/thrift/Exprs.thrift M common/thrift/ExternalDataSource.thrift M common/thrift/Frontend.thrift M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M common/thrift/JniCatalog.thrift M common/thrift/LineageGraph.thrift M common/thrift/Logging.thrift M common/thrift/Metrics.thrift M common/thrift/Partitions.thrift M common/thrift/PlanNodes.thrift M common/thrift/RuntimeProfile.thrift M common/thrift/StatestoreService.thrift M common/thrift/Status.thrift M common/thrift/Types.thrift M common/thrift/beeswax.thrift 20 files changed, 482 insertions(+), 444 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/12543/6 -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 6 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/12543 ) Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. Patch Set 5: (2 comments) One small thing. Otherwise, I'm ready to approve http://gerrit.cloudera.org:8080/#/c/12543/5/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/12543/5/common/thrift/ImpalaService.thrift@61 PS5, Line 61: // > 1: executes on at most that many nodes at any point in time (ie there can be : // more nodes than numNodes with plan fragments for this query but at most : // numNodes would be active at any point in time) : // Constants (NUM_NODES_ALL NUM_NODES_ALL_RACKS) are defined in JavaConstants.thrift. Nit: This change removes a bunch of commas in various comments. Maybe some extra commas got caught in a search-and-replace? http://gerrit.cloudera.org:8080/#/c/12543/5/common/thrift/PlanNodes.thrift File common/thrift/PlanNodes.thrift: http://gerrit.cloudera.org:8080/#/c/12543/5/common/thrift/PlanNodes.thrift@75 PS5, Line 75: // A floating point number in range [0.0 1.0] that gives the probability of denying Same comma missing thing here and other comments in this file. -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 5 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Feb 2019 22:09:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12543 ) Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2202/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 5 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Feb 2019 19:34:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12543 ) Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2200/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 4 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Feb 2019 19:33:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Hello Philip Zeyliger, Tim Armstrong, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12543 to look at the new patch set (#5). Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning IMPALA-7694 added a field in the middle of the Metrics.TUnit enum, which broke backwards compatibility with profiles that had been written by older versions of Impala. This change fixes the ordering by moving the field to the end of the enum. Additionally, it adds a warning to the top of all Thrift files that are part of the binary profile format, and an note of caution to the main definition in RuntimeProfile.thrift. This change also fixes the order of all enums in our Thrift files to make errors like this less likely in the future. Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 --- M common/thrift/BackendGflags.thrift M common/thrift/CatalogObjects.thrift M common/thrift/DataSinks.thrift M common/thrift/ExecStats.thrift M common/thrift/Exprs.thrift M common/thrift/ExternalDataSource.thrift M common/thrift/Frontend.thrift M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M common/thrift/JniCatalog.thrift M common/thrift/LineageGraph.thrift M common/thrift/Logging.thrift M common/thrift/Metrics.thrift M common/thrift/Partitions.thrift M common/thrift/PlanNodes.thrift M common/thrift/RuntimeProfile.thrift M common/thrift/StatestoreService.thrift M common/thrift/Status.thrift M common/thrift/Types.thrift M common/thrift/beeswax.thrift 20 files changed, 557 insertions(+), 519 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/12543/5 -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 5 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12543 ) Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/12543/4/common/thrift/Types.thrift File common/thrift/Types.thrift: http://gerrit.cloudera.org:8080/#/c/12543/4/common/thrift/Types.thrift@107 PS4, Line 107: TESTCASE = 4 > This was added recently for IMPALA-5872. Should we also move it to the end? thanks for catching it, done. -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 4 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Feb 2019 19:02:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/12543 ) Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. Patch Set 4: (1 comment) Thanks for working on this! http://gerrit.cloudera.org:8080/#/c/12543/4/common/thrift/Types.thrift File common/thrift/Types.thrift: http://gerrit.cloudera.org:8080/#/c/12543/4/common/thrift/Types.thrift@107 PS4, Line 107: TESTCASE = 4 This was added recently for IMPALA-5872. Should we also move it to the end? -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 4 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Feb 2019 18:59:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12543 ) Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/12543/3/common/thrift/hive-1-api/TCLIService.thrift File common/thrift/hive-1-api/TCLIService.thrift: PS3: > This is a copy of the Hive thrift so it might cause confusion if we diverge Good point, I reverted it. -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 4 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Feb 2019 18:50:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Hello Philip Zeyliger, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12543 to look at the new patch set (#4). Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning IMPALA-7694 added a field in the middle of the Metrics.TUnit enum, which broke backwards compatibility with profiles that had been written by older versions of Impala. This change fixes the ordering by moving the field to the end of the enum. Additionally, it adds a warning to the top of all Thrift files that are part of the binary profile format, and an note of caution to the main definition in RuntimeProfile.thrift. This change also fixes the order of all enums in our Thrift files to make errors like this less likely in the future. Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 --- M common/thrift/BackendGflags.thrift M common/thrift/CatalogObjects.thrift M common/thrift/DataSinks.thrift M common/thrift/ExecStats.thrift M common/thrift/Exprs.thrift M common/thrift/ExternalDataSource.thrift M common/thrift/Frontend.thrift M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M common/thrift/JniCatalog.thrift M common/thrift/LineageGraph.thrift M common/thrift/Logging.thrift M common/thrift/Metrics.thrift M common/thrift/Partitions.thrift M common/thrift/PlanNodes.thrift M common/thrift/RuntimeProfile.thrift M common/thrift/StatestoreService.thrift M common/thrift/Status.thrift M common/thrift/Types.thrift M common/thrift/beeswax.thrift 20 files changed, 557 insertions(+), 519 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/12543/4 -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 4 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12543 ) Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. Patch Set 3: (1 comment) Looks good, just had the one question about the Hive thrift. http://gerrit.cloudera.org:8080/#/c/12543/3/common/thrift/hive-1-api/TCLIService.thrift File common/thrift/hive-1-api/TCLIService.thrift: PS3: This is a copy of the Hive thrift so it might cause confusion if we diverge. -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 3 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Feb 2019 06:12:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12543 ) Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 3 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Feb 2019 04:00:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12543 ) Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2194/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 3 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Feb 2019 03:05:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12543 ) Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2193/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 2 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Feb 2019 02:59:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Hello Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12543 to look at the new patch set (#3). Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning IMPALA-7694 added a field in the middle of the Metrics.TUnit enum, which broke backwards compatibility with profiles that had been written by older versions of Impala. This change fixes the ordering by moving the field to the end of the enum. Additionally, it adds a warning to the top of all Thrift files that are part of the binary profile format, and an note of caution to the main definition in RuntimeProfile.thrift. This change also fixes the order of all enums in our Thrift files to make errors like this less likely in the future. Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 --- M common/thrift/BackendGflags.thrift M common/thrift/CatalogObjects.thrift M common/thrift/DataSinks.thrift M common/thrift/ExecStats.thrift M common/thrift/Exprs.thrift M common/thrift/ExternalDataSource.thrift M common/thrift/Frontend.thrift M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M common/thrift/JniCatalog.thrift M common/thrift/LineageGraph.thrift M common/thrift/Logging.thrift M common/thrift/Metrics.thrift M common/thrift/Partitions.thrift M common/thrift/PlanNodes.thrift M common/thrift/RuntimeProfile.thrift M common/thrift/StatestoreService.thrift M common/thrift/Status.thrift M common/thrift/Types.thrift M common/thrift/beeswax.thrift M common/thrift/hive-1-api/TCLIService.thrift 21 files changed, 661 insertions(+), 623 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/12543/3 -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 3 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning
Hello Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12543 to look at the new patch set (#2). Change subject: IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning .. IMPALA-8234: Fix ordering of Thrift enum, fix enum values, add warning IMPALA-7694 added a field in the middle of the Metrics.TUnit enum, which broke backwards compatibility with profiles that had been written by older versions of Impala. This change fixes the ordering by moving the field to the end of the enum. Additionally, it adds a warning to the top of all Thrift files that are part of the binary profile format, and an note of caution to the main definition in RuntimeProfile.thrift. This change also fixes the order of all enums in our Thrift files to make errors like this less likely in the future. Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 --- M common/thrift/BackendGflags.thrift M common/thrift/CatalogObjects.thrift M common/thrift/DataSinks.thrift M common/thrift/ExecStats.thrift M common/thrift/Exprs.thrift M common/thrift/ExternalDataSource.thrift M common/thrift/Frontend.thrift M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M common/thrift/JniCatalog.thrift M common/thrift/LineageGraph.thrift M common/thrift/Logging.thrift M common/thrift/Metrics.thrift M common/thrift/Partitions.thrift M common/thrift/PlanNodes.thrift M common/thrift/RuntimeProfile.thrift M common/thrift/StatestoreService.thrift M common/thrift/Status.thrift M common/thrift/Types.thrift M common/thrift/beeswax.thrift M common/thrift/hive-1-api/TCLIService.thrift 21 files changed, 660 insertions(+), 622 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/12543/2 -- To view, visit http://gerrit.cloudera.org:8080/12543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If215f16a636008757ceb439edbd6900a1be88c59 Gerrit-Change-Number: 12543 Gerrit-PatchSet: 2 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
