okumin commented on code in PR #5852:
URL: https://github.com/apache/hive/pull/5852#discussion_r2265895420


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java:
##########
@@ -449,6 +449,53 @@ public Database getDatabase(String catName, String dbName) 
throws MetaException{
     }
   }
 
+  public List<String> getDatabases(String catName, String pattern) throws 
MetaException {
+    List<String> result = new ArrayList<>();
+    StringBuilder queryText = new StringBuilder("SELECT \"NAME\" FROM ")
+            .append(DBS)
+            .append(" WHERE \"CTLG_NAME\" = ?");
+
+    List<Object> params = new ArrayList<>();
+    params.add(catName);
+
+    if (pattern != null && !pattern.equals("*")) {
+      List<String> validPatterns = Arrays.stream(pattern.split("\\|"))
+              .map(String::trim)
+              .filter(s -> !s.isEmpty())
+              .distinct()
+              .map(String::toLowerCase)
+              .toList();
+      if (!validPatterns.isEmpty()) {
+        queryText.append(" AND (");
+        for (int i = 0; i < validPatterns.size(); i++) {
+          if (i > 0) queryText.append(" OR ");
+          queryText.append("\"NAME\" LIKE ?");
+          params.add(validPatterns.get(i).replace('*', '%'));
+        }
+        queryText.append(")");
+      }
+    }
+
+    queryText.append(" ORDER BY \"NAME\" ASC");
+

Review Comment:
   We might want to instantiate and reuse the query text.
   
   ```java
   String properVariableName = queryText.toString();
   ```



##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java:
##########
@@ -449,6 +449,53 @@ public Database getDatabase(String catName, String dbName) 
throws MetaException{
     }
   }
 
+  public List<String> getDatabases(String catName, String pattern) throws 
MetaException {
+    List<String> result = new ArrayList<>();
+    StringBuilder queryText = new StringBuilder("SELECT \"NAME\" FROM ")
+            .append(DBS)
+            .append(" WHERE \"CTLG_NAME\" = ?");
+
+    List<Object> params = new ArrayList<>();
+    params.add(catName);
+
+    if (pattern != null && !pattern.equals("*")) {
+      List<String> validPatterns = Arrays.stream(pattern.split("\\|"))
+              .map(String::trim)
+              .filter(s -> !s.isEmpty())
+              .distinct()
+              .map(String::toLowerCase)
+              .toList();
+      if (!validPatterns.isEmpty()) {
+        queryText.append(" AND (");
+        for (int i = 0; i < validPatterns.size(); i++) {
+          if (i > 0) queryText.append(" OR ");
+          queryText.append("\"NAME\" LIKE ?");
+          params.add(validPatterns.get(i).replace('*', '%'));
+        }

Review Comment:
   Does this loop retain the original semantics? I can see the original 
implementation assumes `pattern` is a regular expression, which is not 
identical to that of SQL's `LIKE`.
   
https://github.com/apache/hive/blob/411d3a49cb7a2720907cf0342f5f77f53dbfed3d/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L2027-L2051



##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -993,8 +993,37 @@ public boolean dropDatabase(String catName, String dbname)
 
   @Override
   public List<String> getDatabases(String catName, String pattern) throws 
MetaException {
+    catName = normalizeIdentifier(catName);
+    try {
+      return getDatabasesInternal(catName, pattern);
+    } catch (NoSuchObjectException e) {
+      throw new MetaException(ExceptionUtils.getStackTrace(e));
+    }
+  }
+
+  private List<String> getDatabasesInternal(String catName, String pattern)
+          throws MetaException, NoSuchObjectException {
+    return new GetHelper<List<String>>(catName, null, pattern, true, true) {

Review Comment:
   I'd say two potential updates on this line.
   First, I suspect this should be `new GetHelper<>(catname, null, null, true, 
true)` because GetHelper can unlikely handle a pattern. @dengzhhu653 We would 
like your advice on this hypothesis.
   Second, if `GetHelper` accepts a pattern, this should be 
`GetHelper<>(catName, pattern, null, true, true)` because the given pattern is 
not that of tables but that of databases.



-- 
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]

Reply via email to