bbeaudreault commented on code in PR #4770:
URL: https://github.com/apache/hbase/pull/4770#discussion_r1082074168
##########
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat2.java:
##########
@@ -653,10 +664,9 @@ static void configureIncrementalLoad(Job job,
List<TableInfo> multiTableInfo,
for (TableInfo tableInfo : multiTableInfo) {
regionLocators.add(tableInfo.getRegionLocator());
- String tn = writeMultipleTables
- ?
tableInfo.getRegionLocator().getName().getNameWithNamespaceInclAsString()
- : tableInfo.getRegionLocator().getName().getNameAsString();
- allTableNames.add(tn);
+ allTableNames.add(writeToTableWithNamespace ?
Review Comment:
Yea I think we need the code I suggested. In 2.0, we only honor the new
config if writeMultipleTables is true. That's what we documented in the comment
on the config key. That's also how the behavior works in the other block in
this file:
```
if (writeMultipleTables) {
// some stuff
tableNameBytes = writeToTableWithNamespace ? one : two;
}
```
That is the same as a logical AND. We need to do the same thing here, but
just using ternary, so:
```
String tn = writeMultipleTables && writeToTableWithNamespace
?
tableInfo.getRegionLocator().getName().getNameWithNamespaceInclAsString()
: tableInfo.getRegionLocator().getName().getNameAsString();
```
That way we are consistent. Otherwise let's say someone sets
writeToTableWithNamespace to true in 2.0, but not writeMultipleTables. When
they upgrade to 3.0, the logic will look like this:
```
String tn = writeMultipleTables
?
tableInfo.getRegionLocator().getName().getNameWithNamespaceInclAsString()
: tableInfo.getRegionLocator().getName().getNameAsString();
```
So since they didn't set writeMultipleTables to true, the output path would
revert to not include namespace.
Does that make sense? I think we just need the && so we are consistent in
this file and with 3.0.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
