aasha commented on a change in pull request #1120:
URL: https://github.com/apache/hive/pull/1120#discussion_r444723985
##########
File path:
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosExternalTables.java
##########
@@ -517,7 +508,7 @@ public void externalTableIncrementalReplication() throws
Throwable {
+ "'")
.run("alter table t1 add partition(country='india')")
.run("alter table t1 add partition(country='us')")
- .dump(primaryDbName, withClause);
Review comment:
why is the withclause removed from here?
##########
File path:
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/BaseReplicationAcrossInstances.java
##########
@@ -103,6 +114,12 @@ public static void classLevelTearDown() throws IOException
{
replica.close();
}
+ private static void setReplicaExternalBase(FileSystem fs, Map<String,
String> confMap) throws IOException {
+ fs.mkdirs(REPLICA_EXTERNAL_BASE);
+ fullyQualifiedReplicaExternalBase =
fs.getFileStatus(REPLICA_EXTERNAL_BASE).getPath().toString();
+ confMap.put(HiveConf.ConfVars.REPL_EXTERNAL_TABLE_BASE_DIR.varname,
fullyQualifiedReplicaExternalBase);
Review comment:
this is set at 104 line also
##########
File path:
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/ReplicationTestUtils.java
##########
@@ -502,19 +502,11 @@ public static void insertForMerge(WarehouseInstance
primary, String primaryDbNam
"creation", "creation", "merge_update", "merge_insert",
"merge_insert"});
}
- public static List<String> externalTableBasePathWithClause(String
replExternalBase, WarehouseInstance replica)
- throws IOException, SemanticException {
- Path externalTableLocation = new Path(replExternalBase);
- DistributedFileSystem fileSystem = replica.miniDFSCluster.getFileSystem();
- externalTableLocation =
PathBuilder.fullyQualifiedHDFSUri(externalTableLocation, fileSystem);
- fileSystem.mkdirs(externalTableLocation);
-
- // this is required since the same filesystem is used in both source and
target
- return Arrays.asList(
- "'" + HiveConf.ConfVars.REPL_EXTERNAL_TABLE_BASE_DIR.varname +
"'='"
- + externalTableLocation.toString() + "'",
- "'distcp.options.pugpb'=''"
- );
+ public static List<String> externalTableClause(boolean enable) {
Review comment:
should this be include external table clause
##########
File path:
ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplExternalTables.java
##########
@@ -66,6 +67,9 @@ private ReplExternalTables(){}
public static String externalTableLocation(HiveConf hiveConf, String
location) throws SemanticException {
String baseDir =
hiveConf.get(HiveConf.ConfVars.REPL_EXTERNAL_TABLE_BASE_DIR.varname);
+ if (StringUtils.isEmpty(baseDir)) {
Review comment:
At the time of load the REPL_EXTERNAL_TABLE_BASE_DIR fully qualified
path is not needed?
##########
File path:
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosExternalTables.java
##########
@@ -503,8 +495,7 @@ public void externalTableIncrementalCheckpointing() throws
Throwable {
@Test
public void externalTableIncrementalReplication() throws Throwable {
- List<String> withClause = externalTableBasePathWithClause();
- WarehouseInstance.Tuple tuple = primary.dump(primaryDbName, withClause);
+ WarehouseInstance.Tuple tuple = primary.dump(primaryDbName);
Review comment:
with clause removed?
##########
File path:
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosExternalTables.java
##########
@@ -159,14 +154,14 @@ public void externalTableReplicationWithDefaultPaths()
throws Throwable {
.run("insert into table t2 partition(country='india') values
('bangalore')")
.run("insert into table t2 partition(country='us') values ('austin')")
.run("insert into table t2 partition(country='france') values
('paris')")
- .dump(primaryDbName, withClauseOptions);
+ .dump(primaryDbName);
Review comment:
why is the with clause removed?
##########
File path:
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosExternalTables.java
##########
@@ -623,7 +612,7 @@ public void bootstrapExternalTablesDuringIncrementalPhase()
throws Throwable {
assertFalse(primary.miniDFSCluster.getFileSystem()
.exists(new Path(metadataPath + relativeExtInfoPath(null))));
- replica.load(replicatedDbName, primaryDbName, loadWithClause)
+ replica.load(replicatedDbName, primaryDbName)
Review comment:
with clause removed?
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java
##########
@@ -750,10 +752,19 @@ private void dumpTableListToDumpLocation(List<String>
tableList, Path dbRoot, St
private List<DirCopyWork> dirLocationsToCopy(List<Path> sourceLocations)
throws HiveException {
+ if (sourceLocations.isEmpty()) {
+ return Collections.emptyList();
+ }
List<DirCopyWork> list = new ArrayList<>(sourceLocations.size());
String baseDir =
conf.get(HiveConf.ConfVars.REPL_EXTERNAL_TABLE_BASE_DIR.varname);
// this is done to remove any scheme related information that will be
present in the base path
// specifically when we are replicating to cloud storage
+ URI basePathUri = StringUtils.isEmpty(baseDir) ? null : new
Path(baseDir).toUri();
+ if (basePathUri == null || basePathUri.getScheme() == null ||
basePathUri.getAuthority() == null) {
Review comment:
Can be moved to a method
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
