You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "ngsg (via GitHub)" <gi...@apache.org> on 2025/08/29 10:24:42 UTC
[PR] HIVE-29166: Fix the partition column update logic in ConvertJoinMapJoin#convertJoinBucketMapJoin. [hive]
ngsg opened a new pull request, #6048:
URL: https://github.com/apache/hive/pull/6048
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
Modify the partition column update logic in `ConvertJoinMapJoin#convertJoinBucketMapJoin` so that small table's partitionCols are updated if they do not match the big table's bucketCols.
Previously, the partition columns were updated only when the number of partition columns is not equal to the number of bucket columns. However, this could cause incorrect bucket routing when the counts are equal but the column orders differ. For example, if the bucket columns are {id, part} and the partition columns are {part, id}, then the partition columns should be updated to {id, part}, but the current implementation does not update them.
The new logic now updates the partition columns if the counts are not equal or the column orders differ.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
As described above, the current BucketMapJoin conversion logic may not update the partition columns properly, which leads to incorrect results, as reported in the JIRA.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
I added two qfiles in which the current Hive returns incorrect results. I also checked that the patch fixes the originally reported duplicate-generation issue.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29166: Fix the partition column update logic in ConvertJoinMapJoin#convertJoinBucketMapJoin. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6048:
URL: https://github.com/apache/hive/pull/6048#discussion_r2324194808
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java:
##########
@@ -656,32 +656,39 @@ private boolean convertJoinBucketMapJoin(JoinOperator joinOp, OptimizeTezProcCon
// on small table(s).
ReduceSinkOperator bigTableRS = (ReduceSinkOperator)joinOp.getParentOperators().get(bigTablePosition);
OpTraits opTraits = bigTableRS.getOpTraits();
- List<List<String>> listBucketCols = opTraits.getBucketColNames();
+ // It is guaranteed there is only 1 list within bigTableRS.getOpTraits().getBucketColNames().
+ List<String> listBucketCols = opTraits.getBucketColNames().get(0);
List<ExprNodeDesc> bigTablePartitionCols = bigTableRS.getConf().getPartitionCols();
- boolean updatePartitionCols = false;
+ boolean updatePartitionCols = listBucketCols.size() != bigTablePartitionCols.size();
List<Integer> positions = new ArrayList<>();
- CustomBucketFunction bucketFunction = opTraits.getCustomBucketFunctions().get(0);
- if (listBucketCols.get(0).size() != bigTablePartitionCols.size()) {
- updatePartitionCols = true;
- // Prepare updated partition columns for small table(s).
- // Get the positions of bucketed columns
-
- int bigTableExprPos = 0;
- Map<String, ExprNodeDesc> colExprMap = bigTableRS.getColumnExprMap();
- final boolean[] retainedColumns = new boolean[listBucketCols.get(0).size()];
- for (ExprNodeDesc bigTableExpr : bigTablePartitionCols) {
- // It is guaranteed there is only 1 list within listBucketCols.
- for (int i = 0; i < listBucketCols.get(0).size(); i++) {
- final String colName = listBucketCols.get(0).get(i);
- if (colExprMap.get(colName).isSame(bigTableExpr)) {
- positions.add(bigTableExprPos);
- retainedColumns[i] = true;
- }
+ // Compare the partition columns and the bucket columns of bigTableRS.
+ Map<String, ExprNodeDesc> colExprMap = bigTableRS.getColumnExprMap();
+ final boolean[] retainedColumns = new boolean[listBucketCols.size()];
+ for (int bucketColIdx = 0; bucketColIdx < listBucketCols.size(); bucketColIdx++) {
+ for (int bigTablePartIdx = 0; bigTablePartIdx < bigTablePartitionCols.size(); bigTablePartIdx++) {
+ ExprNodeDesc bigTablePartExpr = bigTablePartitionCols.get(bigTablePartIdx);
+ ExprNodeDesc bucketColExpr = colExprMap.get(listBucketCols.get(bucketColIdx));
+ if (bigTablePartExpr.isSame(bucketColExpr)) {
+ positions.add(bigTablePartIdx);
+ retainedColumns[bucketColIdx] = true;
+ // If the positions of the partition column and the bucket column are not the same,
+ // then we need to update the position of the partition column in small tables.
+ updatePartitionCols = updatePartitionCols || bucketColIdx != bigTablePartIdx;
+ break;
}
- bigTableExprPos = bigTableExprPos + 1;
}
+ }
+ // If the number of partition columns is less than the number of bucket columns,
+ // then we cannot properly distribute small tables onto bucketized map tasks.
+ // Bail out.
+ if (positions.size() < listBucketCols.size()) {
+ return false;
+ }
+
+ CustomBucketFunction bucketFunction = opTraits.getCustomBucketFunctions().get(0);
Review Comment:
@ngsg, @okumin shouldn't we apply `retainedColumns` to a `bucketFunction` even when `updatePartitionCols` is false?
````
final Optional<CustomBucketFunction> selected =
opTraits.getCustomBucketFunctions().get(0).select(retainedColumns);
if (!selected.isPresent()) {
LOG.info("{} can't keep itself only with {}", opTraits.getCustomBucketFunctions().get(0), retainedColumns);
return false;
}
bucketFunction = selected.get();
````
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29166: Fix the partition column update logic in ConvertJoinMapJoin#convertJoinBucketMapJoin. [hive]
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on code in PR #6048:
URL: https://github.com/apache/hive/pull/6048#discussion_r2312973032
##########
ql/src/test/results/clientpositive/llap/bucketmapjoin14.q.out:
##########
@@ -0,0 +1,112 @@
+PREHOOK: query: CREATE TABLE tbl (foid string, part string, id string) CLUSTERED BY (id, part) INTO 64 BUCKETS
+PREHOOK: type: CREATETABLE
+PREHOOK: Output: database:default
+PREHOOK: Output: default@tbl
+POSTHOOK: query: CREATE TABLE tbl (foid string, part string, id string) CLUSTERED BY (id, part) INTO 64 BUCKETS
+POSTHOOK: type: CREATETABLE
+POSTHOOK: Output: database:default
+POSTHOOK: Output: default@tbl
+PREHOOK: query: INSERT INTO tbl VALUES ('1234', 'PART_123', '1'), ('1235', 'PART_124', '2')
+PREHOOK: type: QUERY
+PREHOOK: Input: _dummy_database@_dummy_table
+PREHOOK: Output: default@tbl
+POSTHOOK: query: INSERT INTO tbl VALUES ('1234', 'PART_123', '1'), ('1235', 'PART_124', '2')
+POSTHOOK: type: QUERY
+POSTHOOK: Input: _dummy_database@_dummy_table
+POSTHOOK: Output: default@tbl
+POSTHOOK: Lineage: tbl.foid SCRIPT []
+POSTHOOK: Lineage: tbl.id SCRIPT []
+POSTHOOK: Lineage: tbl.part SCRIPT []
+PREHOOK: query: EXPLAIN
+SELECT * FROM tbl JOIN tbl tbl2 ON tbl.id = tbl2.id AND tbl.part = tbl2.part
+PREHOOK: type: QUERY
+PREHOOK: Input: default@tbl
+#### A masked pattern was here ####
+POSTHOOK: query: EXPLAIN
+SELECT * FROM tbl JOIN tbl tbl2 ON tbl.id = tbl2.id AND tbl.part = tbl2.part
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@tbl
+#### A masked pattern was here ####
+STAGE DEPENDENCIES:
+ Stage-1 is a root stage
+ Stage-0 depends on stages: Stage-1
+
+STAGE PLANS:
+ Stage: Stage-1
+ Tez
+#### A masked pattern was here ####
+ Edges:
+ Map 1 <- Map 2 (CUSTOM_EDGE)
+#### A masked pattern was here ####
+ Vertices:
+ Map 1
+ Map Operator Tree:
+ TableScan
+ alias: tbl
+ filterExpr: (id is not null and part is not null) (type: boolean)
+ Statistics: Num rows: 2 Data size: 530 Basic stats: COMPLETE Column stats: COMPLETE
+ Filter Operator
+ predicate: (id is not null and part is not null) (type: boolean)
+ Statistics: Num rows: 2 Data size: 530 Basic stats: COMPLETE Column stats: COMPLETE
+ Select Operator
+ expressions: foid (type: string), part (type: string), id (type: string)
+ outputColumnNames: _col0, _col1, _col2
+ Statistics: Num rows: 2 Data size: 530 Basic stats: COMPLETE Column stats: COMPLETE
+ Map Join Operator
+ condition map:
+ Inner Join 0 to 1
+ keys:
+ 0 _col1 (type: string), _col2 (type: string)
+ 1 _col1 (type: string), _col2 (type: string)
+ outputColumnNames: _col0, _col1, _col2, _col3, _col4, _col5
+ input vertices:
+ 1 Map 2
+ Statistics: Num rows: 2 Data size: 1060 Basic stats: COMPLETE Column stats: COMPLETE
+ File Output Operator
+ compressed: false
+ Statistics: Num rows: 2 Data size: 1060 Basic stats: COMPLETE Column stats: COMPLETE
+ table:
+ input format: org.apache.hadoop.mapred.SequenceFileInputFormat
+ output format: org.apache.hadoop.hive.ql.io.HiveSequenceFileOutputFormat
+ serde: org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe
+ Execution mode: vectorized, llap
+ LLAP IO: all inputs
+ Map 2
+ Map Operator Tree:
+ TableScan
+ alias: tbl2
+ filterExpr: (id is not null and part is not null) (type: boolean)
+ Statistics: Num rows: 2 Data size: 530 Basic stats: COMPLETE Column stats: COMPLETE
+ Filter Operator
+ predicate: (id is not null and part is not null) (type: boolean)
+ Statistics: Num rows: 2 Data size: 530 Basic stats: COMPLETE Column stats: COMPLETE
+ Select Operator
+ expressions: foid (type: string), part (type: string), id (type: string)
+ outputColumnNames: _col0, _col1, _col2
+ Statistics: Num rows: 2 Data size: 530 Basic stats: COMPLETE Column stats: COMPLETE
+ Reduce Output Operator
+ key expressions: _col1 (type: string), _col2 (type: string)
+ null sort order: zz
+ sort order: ++
+ Map-reduce partition columns: _col2 (type: string), _col1 (type: string)
Review Comment:
I verified that this list is inverted on the master branch, and the result would be empty.
```
Map-reduce partition columns: _col1 (type: string), _col2 (type: string)
```
##########
iceberg/iceberg-handler/src/test/results/positive/bucket_map_join_9.q.out:
##########
@@ -0,0 +1,65 @@
+PREHOOK: query: CREATE TABLE tbl (foid string, part string, id string) PARTITIONED BY SPEC(bucket(10, id), bucket(10, part)) STORED BY ICEBERG
+PREHOOK: type: CREATETABLE
+PREHOOK: Output: database:default
+PREHOOK: Output: default@tbl
+POSTHOOK: query: CREATE TABLE tbl (foid string, part string, id string) PARTITIONED BY SPEC(bucket(10, id), bucket(10, part)) STORED BY ICEBERG
+POSTHOOK: type: CREATETABLE
+POSTHOOK: Output: database:default
+POSTHOOK: Output: default@tbl
+PREHOOK: query: INSERT INTO tbl VALUES ('1234', 'PART_123', '1'), ('1235', 'PART_124', '2')
+PREHOOK: type: QUERY
+PREHOOK: Input: _dummy_database@_dummy_table
+PREHOOK: Output: default@tbl
+POSTHOOK: query: INSERT INTO tbl VALUES ('1234', 'PART_123', '1'), ('1235', 'PART_124', '2')
+POSTHOOK: type: QUERY
+POSTHOOK: Input: _dummy_database@_dummy_table
+POSTHOOK: Output: default@tbl
+PREHOOK: query: EXPLAIN
+SELECT * FROM tbl JOIN tbl tbl2 ON tbl.id = tbl2.id AND tbl.part = tbl2.part
+PREHOOK: type: QUERY
+PREHOOK: Input: default@tbl
+PREHOOK: Output: hdfs://### HDFS PATH ###
+POSTHOOK: query: EXPLAIN
+SELECT * FROM tbl JOIN tbl tbl2 ON tbl.id = tbl2.id AND tbl.part = tbl2.part
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@tbl
+POSTHOOK: Output: hdfs://### HDFS PATH ###
+Plan optimized by CBO.
+
+Vertex dependency in root stage
+Map 1 <- Map 2 (CUSTOM_EDGE)
+
+Stage-0
+ Fetch Operator
+ limit:-1
+ Stage-1
+ Map 1 vectorized
+ File Output Operator [FS_53]
+ Map Join Operator [MAPJOIN_52] (rows=2 width=530)
+ BucketMapJoin:true,Conds:SEL_51._col1, _col2=RS_49._col1, _col2(Inner),Output:["_col0","_col1","_col2","_col3","_col4","_col5"]
+ <-Map 2 [CUSTOM_EDGE] vectorized
+ MULTICAST [RS_49]
+ PartitionCols:_col2, _col1
Review Comment:
I verified that this list is inverted on the master branch, and the result would be empty.
```
PartitionCols:_col1, _col2
```
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29166: Fix the partition column update logic in ConvertJoinMapJoin#convertJoinBucketMapJoin. [hive]
Posted by "aturoczy (via GitHub)" <gi...@apache.org>.
aturoczy commented on PR #6048:
URL: https://github.com/apache/hive/pull/6048#issuecomment-3254077947
Thank you for the review. If it is OK, can we merge it to master?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29166: Fix the partition column update logic in ConvertJoinMapJoin#convertJoinBucketMapJoin. [hive]
Posted by "difin (via GitHub)" <gi...@apache.org>.
difin commented on PR #6048:
URL: https://github.com/apache/hive/pull/6048#issuecomment-3250492543
LGTM +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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29166: Fix the partition column update logic in ConvertJoinMapJoin#convertJoinBucketMapJoin. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6048:
URL: https://github.com/apache/hive/pull/6048#issuecomment-3236747199
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6048) **Quality Gate passed**
Issues
 [100 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6048&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6048&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6048&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6048&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6048&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6048)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29166: Fix the partition column update logic in ConvertJoinMapJoin#convertJoinBucketMapJoin. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6048:
URL: https://github.com/apache/hive/pull/6048#discussion_r2324194808
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java:
##########
@@ -656,32 +656,39 @@ private boolean convertJoinBucketMapJoin(JoinOperator joinOp, OptimizeTezProcCon
// on small table(s).
ReduceSinkOperator bigTableRS = (ReduceSinkOperator)joinOp.getParentOperators().get(bigTablePosition);
OpTraits opTraits = bigTableRS.getOpTraits();
- List<List<String>> listBucketCols = opTraits.getBucketColNames();
+ // It is guaranteed there is only 1 list within bigTableRS.getOpTraits().getBucketColNames().
+ List<String> listBucketCols = opTraits.getBucketColNames().get(0);
List<ExprNodeDesc> bigTablePartitionCols = bigTableRS.getConf().getPartitionCols();
- boolean updatePartitionCols = false;
+ boolean updatePartitionCols = listBucketCols.size() != bigTablePartitionCols.size();
List<Integer> positions = new ArrayList<>();
- CustomBucketFunction bucketFunction = opTraits.getCustomBucketFunctions().get(0);
- if (listBucketCols.get(0).size() != bigTablePartitionCols.size()) {
- updatePartitionCols = true;
- // Prepare updated partition columns for small table(s).
- // Get the positions of bucketed columns
-
- int bigTableExprPos = 0;
- Map<String, ExprNodeDesc> colExprMap = bigTableRS.getColumnExprMap();
- final boolean[] retainedColumns = new boolean[listBucketCols.get(0).size()];
- for (ExprNodeDesc bigTableExpr : bigTablePartitionCols) {
- // It is guaranteed there is only 1 list within listBucketCols.
- for (int i = 0; i < listBucketCols.get(0).size(); i++) {
- final String colName = listBucketCols.get(0).get(i);
- if (colExprMap.get(colName).isSame(bigTableExpr)) {
- positions.add(bigTableExprPos);
- retainedColumns[i] = true;
- }
+ // Compare the partition columns and the bucket columns of bigTableRS.
+ Map<String, ExprNodeDesc> colExprMap = bigTableRS.getColumnExprMap();
+ final boolean[] retainedColumns = new boolean[listBucketCols.size()];
+ for (int bucketColIdx = 0; bucketColIdx < listBucketCols.size(); bucketColIdx++) {
+ for (int bigTablePartIdx = 0; bigTablePartIdx < bigTablePartitionCols.size(); bigTablePartIdx++) {
+ ExprNodeDesc bigTablePartExpr = bigTablePartitionCols.get(bigTablePartIdx);
+ ExprNodeDesc bucketColExpr = colExprMap.get(listBucketCols.get(bucketColIdx));
+ if (bigTablePartExpr.isSame(bucketColExpr)) {
+ positions.add(bigTablePartIdx);
+ retainedColumns[bucketColIdx] = true;
+ // If the positions of the partition column and the bucket column are not the same,
+ // then we need to update the position of the partition column in small tables.
+ updatePartitionCols = updatePartitionCols || bucketColIdx != bigTablePartIdx;
+ break;
}
- bigTableExprPos = bigTableExprPos + 1;
}
+ }
+ // If the number of partition columns is less than the number of bucket columns,
+ // then we cannot properly distribute small tables onto bucketized map tasks.
+ // Bail out.
+ if (positions.size() < listBucketCols.size()) {
+ return false;
+ }
+
+ CustomBucketFunction bucketFunction = opTraits.getCustomBucketFunctions().get(0);
Review Comment:
@ngsg, @okumin shouldn't we apply `retainedColumns` to a `bucketFunction`?
````
final Optional<CustomBucketFunction> selected =
opTraits.getCustomBucketFunctions().get(0).select(retainedColumns);
if (!selected.isPresent()) {
LOG.info("{} can't keep itself only with {}", opTraits.getCustomBucketFunctions().get(0), retainedColumns);
return false;
}
bucketFunction = selected.get();
````
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29166: Fix the partition column update logic in ConvertJoinMapJoin#convertJoinBucketMapJoin. [hive]
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz merged PR #6048:
URL: https://github.com/apache/hive/pull/6048
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29166: Fix the partition column update logic in ConvertJoinMapJoin#convertJoinBucketMapJoin. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6048:
URL: https://github.com/apache/hive/pull/6048#issuecomment-3237767006
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6048) **Quality Gate passed**
Issues
 [100 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6048&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6048&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6048&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6048&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6048&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6048)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29166: Fix the partition column update logic in ConvertJoinMapJoin#convertJoinBucketMapJoin. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6048:
URL: https://github.com/apache/hive/pull/6048#discussion_r2324194808
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java:
##########
@@ -656,32 +656,39 @@ private boolean convertJoinBucketMapJoin(JoinOperator joinOp, OptimizeTezProcCon
// on small table(s).
ReduceSinkOperator bigTableRS = (ReduceSinkOperator)joinOp.getParentOperators().get(bigTablePosition);
OpTraits opTraits = bigTableRS.getOpTraits();
- List<List<String>> listBucketCols = opTraits.getBucketColNames();
+ // It is guaranteed there is only 1 list within bigTableRS.getOpTraits().getBucketColNames().
+ List<String> listBucketCols = opTraits.getBucketColNames().get(0);
List<ExprNodeDesc> bigTablePartitionCols = bigTableRS.getConf().getPartitionCols();
- boolean updatePartitionCols = false;
+ boolean updatePartitionCols = listBucketCols.size() != bigTablePartitionCols.size();
List<Integer> positions = new ArrayList<>();
- CustomBucketFunction bucketFunction = opTraits.getCustomBucketFunctions().get(0);
- if (listBucketCols.get(0).size() != bigTablePartitionCols.size()) {
- updatePartitionCols = true;
- // Prepare updated partition columns for small table(s).
- // Get the positions of bucketed columns
-
- int bigTableExprPos = 0;
- Map<String, ExprNodeDesc> colExprMap = bigTableRS.getColumnExprMap();
- final boolean[] retainedColumns = new boolean[listBucketCols.get(0).size()];
- for (ExprNodeDesc bigTableExpr : bigTablePartitionCols) {
- // It is guaranteed there is only 1 list within listBucketCols.
- for (int i = 0; i < listBucketCols.get(0).size(); i++) {
- final String colName = listBucketCols.get(0).get(i);
- if (colExprMap.get(colName).isSame(bigTableExpr)) {
- positions.add(bigTableExprPos);
- retainedColumns[i] = true;
- }
+ // Compare the partition columns and the bucket columns of bigTableRS.
+ Map<String, ExprNodeDesc> colExprMap = bigTableRS.getColumnExprMap();
+ final boolean[] retainedColumns = new boolean[listBucketCols.size()];
+ for (int bucketColIdx = 0; bucketColIdx < listBucketCols.size(); bucketColIdx++) {
+ for (int bigTablePartIdx = 0; bigTablePartIdx < bigTablePartitionCols.size(); bigTablePartIdx++) {
+ ExprNodeDesc bigTablePartExpr = bigTablePartitionCols.get(bigTablePartIdx);
+ ExprNodeDesc bucketColExpr = colExprMap.get(listBucketCols.get(bucketColIdx));
+ if (bigTablePartExpr.isSame(bucketColExpr)) {
+ positions.add(bigTablePartIdx);
+ retainedColumns[bucketColIdx] = true;
+ // If the positions of the partition column and the bucket column are not the same,
+ // then we need to update the position of the partition column in small tables.
+ updatePartitionCols = updatePartitionCols || bucketColIdx != bigTablePartIdx;
+ break;
}
- bigTableExprPos = bigTableExprPos + 1;
}
+ }
+ // If the number of partition columns is less than the number of bucket columns,
+ // then we cannot properly distribute small tables onto bucketized map tasks.
+ // Bail out.
+ if (positions.size() < listBucketCols.size()) {
+ return false;
+ }
+
+ CustomBucketFunction bucketFunction = opTraits.getCustomBucketFunctions().get(0);
Review Comment:
@ngsg, @okumin shouldn't we apply `retainedColumns` to a `bucketFunction` even when `updatePartitionCols` is false?
````
final Optional<CustomBucketFunction> selected =
opTraits.getCustomBucketFunctions().get(0).select(retainedColumns);
if (!selected.isPresent()) {
LOG.info("{} can't keep itself only with {}", opTraits.getCustomBucketFunctions().get(0), retainedColumns);
return false;
}
bucketFunction = selected.get();
````
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org