apurtell commented on code in PR #4770: URL: https://github.com/apache/hbase/pull/4770#discussion_r988469107
########## hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat2.java: ########## @@ -254,16 +255,19 @@ public void write(ImmutableBytesWritable row, V cell) throws IOException { byte[] tableNameBytes = null; if (writeMultipleTables) { tableNameBytes = MultiTableHFileOutputFormat.getTableName(row.get()); - tableNameBytes = TableName.valueOf(tableNameBytes).toBytes(); + tableNameBytes = TableName.valueOf(tableNameBytes).getNameWithNamespaceInclAsString() Review Comment: Applications (HFile readers) that read things written by HFileOutputFormat2 may need to change. There is a path building semantic change implied by `getNameWithNamespaceInclAsString` being necessary now for something introduced by this patch. Is this a compatibility problem? Are these changes necessary? ########## hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat2.java: ########## @@ -651,15 +651,18 @@ private void doIncrementalLoadTest(boolean shouldChangeRegions, boolean shouldKe Path testDir = util.getDataTestDirOnTestFS("testLocalMRIncrementalLoad"); // Generate the bulk load files runIncrementalPELoad(conf, tableInfo, testDir, putSortReducer); + if (writeMultipleTables) { Review Comment: This is what I mean. The test has to be updated, so readers/consumers of HFileOutputFormat2 produced output will need to change too? Does this fall under the operational compatibility guidelines? -- 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: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org