> On jún. 27, 2018, 12:53 du, Peter Vary wrote: > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/FileUtils.java > > Lines 526 (patched) > > <https://reviews.apache.org/r/67753/diff/2/?file=2046288#file2046288line526> > > > > Are we sure that the location is not null at this stage? > > Oleksiy Sayankin wrote: > Saying _location_, do you mean variable _path_ here or the result of > _getFileSystem(conf)_? > > Peter Vary wrote: > Path... sorry for the confusion! > > Oleksiy Sayankin wrote: > Yes. From sources: > > Path dbPath = wh.determineDatabasePath(cat, db); > String dbLocationUri = wh.getDnsPath(dbPath).toString(); > Configuration conf = getConf(); > if (!MetastoreConf.getBoolVar(conf, > ConfVars.METASTORE_ALLOW_NEW_DB_IN_EXISTING_DIRECTORY) && FileUtils > .exists(dbPath, conf)) { > > where method _determineDatabasePath()_ is: > > > public Path determineDatabasePath(Catalog cat, Database db) throws > MetaException { > if (db.isSetLocationUri()) { > return getDnsPath(new Path(db.getLocationUri())); > } > if (cat == null || > cat.getName().equalsIgnoreCase(DEFAULT_CATALOG_NAME)) { > if (db.getName().equalsIgnoreCase(DEFAULT_DATABASE_NAME)) { > return getWhRoot(); > } else { > return new Path(getWhRoot(), dbDirFromDbName(db)); > } > } else { > return new Path(getDnsPath(new Path(cat.getLocationUri())), > dbDirFromDbName(db)); > } > } > > look at each __return__ statement. Every time we use __new Path()__ > directly or as an argument to a method _getDnsPath()_. And it also uses > __return new Path()__. See > > public static Path getDnsPath(Path path, Configuration conf) throws > MetaException { > FileSystem fs = getFs(path, conf); > return (new Path(fs.getUri().getScheme(), fs.getUri().getAuthority(), > path > .toUri().getPath())); > } > > So _dbPath_ can't be __null__ here. Hence > > return path.getFileSystem(conf).exists(path); > > works fine and there is no NPE.
Thanks for the explanation! Cool, then add some comment javadoc for the new public method (FileUtils.exists), that the path should not be null, and I think we should "only" wait for the precommit/yetus test results :) - Peter ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67753/#review205424 ----------------------------------------------------------- On jún. 27, 2018, 1:23 du, Oleksiy Sayankin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67753/ > ----------------------------------------------------------- > > (Updated jún. 27, 2018, 1:23 du) > > > Review request for hive, Zoltan Haindrich and Zoltan Haindrich. > > > Repository: hive-git > > > Description > ------- > > Added hive.metastore.allow.new.db.in.existing.directory property > > > Diffs > ----- > > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > e9d7e7c397 > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java > f3b909ca44 > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/FileUtils.java > 963e12f9d8 > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestMetaCreateObjects.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/67753/diff/4/ > > > Testing > ------- > > > Thanks, > > Oleksiy Sayankin > >
