Copilot commented on code in PR #6717:
URL: https://github.com/apache/hive/pull/6717#discussion_r3955830870
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/CreateDatabaseHandler.java:
##########
@@ -269,4 +241,44 @@ public record CreateDatabaseResult(boolean success,
Map<String, String>
transactionalListenersResponses) implements Result {
}
+
+ /**
+ * Creates the given database directory (managed or external) as the given
user,
+ * running the actual mkdir as an admin (login) or current user depending on
runAsLoginUser.
+ *
+ * @param path the directory path to create
+ * @param runAsLoginUser true to run as the login (admin) user (used for
managed dir,
+ * since the calling user may not have access to it),
+ * false to run as the current user (used for
external dir)
+ * @param dirLabel a short label ("managed"/"external") used only for
log/error messages
+ * @param throwOnMkdirFailure true to throw the exception about create
database dir
+ * @return true if the directory was created by this call, false if it
already existed
+ * @throws MetaException if directory creation fails
+ */
+ private boolean createDbDirectory(Path path, boolean runAsLoginUser, String
dirLabel,
+ boolean throwOnMkdirFailure) throws
MetaException {
+ try {
+ UserGroupInformation ugi = runAsLoginUser
+ ? UserGroupInformation.getLoginUser()
+ : UserGroupInformation.getCurrentUser();
+ return ugi.doAs((PrivilegedExceptionAction<Boolean>) () -> {
+ if (!wh.isDir(path)) {
+ LOG.info("Creating database path in {} directory {}", dirLabel,
path);
+ if (!wh.mkdirs(path)) {
+ if (throwOnMkdirFailure) {
+ throw new MetaException("Unable to create database " + dirLabel
+ " path " + path +
+ ", failed to create database " + db.getName());
+ }
+ return false;
+ }
+ return true;
+ }
+ return false;
+ });
+ } catch (IOException | InterruptedException | UndeclaredThrowableException
e) {
+ throw new MetaException("Unable to create database " + dirLabel + "
directory " + path +
+ ", failed to create database " + db.getName() + ": " +
e.getMessage());
+ }
Review Comment:
The helper’s catch block wraps all failures into a new MetaException using
e.getMessage(), which can lose the original error details (especially when a
MetaException thrown inside doAs is wrapped in an UndeclaredThrowableException)
and also drops the more actionable external-path context that previously warned
about StorageBasedAuthorizationProvider. Consider unwrapping
UndeclaredThrowableException (rethrow the original MetaException when present)
and tailoring the external-path error message to retain the prior guidance.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
