From 4745648dd047bb7c9b96c6db95ef106ff99b5ecc Mon Sep 17 00:00:00 2001 From: Alexander Kolbasov Date: Mon, 16 Oct 2017 09:05:37 -0700 Subject: [PATCH] HIVE-17730 Queries can be closed automatically --- .../hadoop/hive/metastore/tools/HiveMetaTool.java | 21 +- .../apache/hadoop/hive/metastore/ObjectStore.java | 708 ++++++++++----------- 2 files changed, 357 insertions(+), 372 deletions(-) diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java b/metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java index 22e246f1c9..e8393b66ec 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java @@ -149,21 +149,14 @@ private void executeJDOQLSelect(String query) { initObjectStore(hiveConf); System.out.println("Executing query: " + query); - ObjectStore.QueryWrapper queryWrapper = new ObjectStore.QueryWrapper(); - try { - Collection result = objStore.executeJDOQLSelect(query, queryWrapper); - if (result != null) { - Iterator iter = result.iterator(); - while (iter.hasNext()) { - Object o = iter.next(); - System.out.println(o.toString()); - } - } else { - System.err.println("Encountered error during executeJDOQLSelect -" + - "commit of JDO transaction failed."); + Collection result = objStore.executeJDOQLSelect(query); + if (result != null) { + for (Object o : result) { + System.out.println(o.toString()); } - } finally { - queryWrapper.close(); + } else { + System.err.println("Encountered error during executeJDOQLSelect -" + + "commit of JDO transaction failed."); } } diff --git a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java index ffb2abdf62..e5f54b5206 100644 --- a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java +++ b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java @@ -186,7 +186,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Lists; -import com.google.common.collect.Maps; /** @@ -257,21 +256,107 @@ private Counter directSqlErrors; /** - * A Autocloseable wrapper around Query class to pass the Query object to the caller and let the caller release - * the resources when the QueryWrapper goes out of scope + * AutoCloseable query wrapper which doesn't throw exceptions on close. + * In current JDO implementation Query itself is AutoCloseable, + * but its close() method throws exceptions. To avoid propafating these up the call stack, + * this wrapper provides close() method that doesn't throw exceptions.

+ * + * QueryWrapper simulates regular query operations. New ones may be added as needed. + * The typical access pattern is

+ * + *

+   *   try (QueryWrapper query = new QueryWrapper(pm.newQuery(Foo.class))) {
+   *     return query.execute();
+   *   }
+   * 
+ *

+ * Note that calling explicit close() is never useful because it can be called + * on the query itself instead. */ public static class QueryWrapper implements AutoCloseable { - public Query query; + private final Query query; /** - * Explicitly closes the query object to release the resources + * Initialize from existing query. + * @param query open query. */ + QueryWrapper(Query query) { + this.query = query; + } + + public Query getQuery() { + return query; + } + + // Wrappers around regular Query operations + + long deletePersistentAll(Object... parameters) { + return query.deletePersistentAll(parameters); + } + + void declareParameters(String parameters) { + query.declareParameters(parameters); + } + + void declareImports(String imports) { + query.declareImports(imports); + } + + Object execute() { + return query.execute(); + } + + Object execute(Object p1) { + return query.execute(p1); + } + + Object execute(Object p1, Object p2) { + return query.execute(p1, p2); + } + + Object execute(Object p1, Object p2, Object p3) { + return query.execute(p1, p2, p3); + } + + Object executeWithMap (Map parameters) { + return query.executeWithMap(parameters); + } + + Object executeWithArray (Object... parameters) { + return query.executeWithArray(parameters); + } + + void setFilter(String filter) { + query.setFilter(filter); + } + + void setOrdering(String ordering) { + query.setOrdering(ordering); + } + + void setRange(long fromIncl, long toExcl) { + query.setRange(fromIncl, toExcl); + } + + void setResult(String data) { + query.setResult(data); + } + + void setUnique(boolean unique) { + query.setUnique(unique); + } + + void setResultClass(Class cls) { + query.setResultClass(cls); + } + + void setClass(Class cls) { + query.setClass(cls); + } + @Override public void close() { - if (query != null) { - query.closeAll(); - query = null; - } + query.closeAll(); } } @@ -903,10 +988,9 @@ public boolean alterDatabase(String dbName, Database db) @Override public boolean dropDatabase(String dbname) throws NoSuchObjectException, MetaException { - boolean success = false; - LOG.info("Dropping database " + dbname + " along with all tables"); + boolean committed = false; + LOG.info("Dropping database {} along with all tables", dbname); dbname = normalizeIdentifier(dbname); - QueryWrapper queryWrapper = new QueryWrapper(); try { openTransaction(); @@ -914,17 +998,19 @@ public boolean dropDatabase(String dbname) throws NoSuchObjectException, MetaExc MDatabase db = getMDatabase(dbname); pm.retrieve(db); if (db != null) { - List dbGrants = this.listDatabaseGrants(dbname, queryWrapper); - if (dbGrants != null && dbGrants.size() > 0) { + List dbGrants = this.listDatabaseGrants(dbname); + if (dbGrants != null && !dbGrants.isEmpty()) { pm.deletePersistentAll(dbGrants); } pm.deletePersistent(db); } - success = commitTransaction(); + committed = commitTransaction(); } finally { - rollbackAndCleanup(success, queryWrapper); + if (!committed) { + rollbackTransaction(); + } } - return success; + return committed; } @Override @@ -2228,12 +2314,7 @@ private boolean dropPartitionCommon(MPartition part) throws NoSuchObjectExceptio @Override protected List getJdoResult( GetHelper> ctx) throws MetaException { - QueryWrapper queryWrapper = new QueryWrapper(); - try { - return convertToParts(listMPartitions(dbName, tblName, maxParts, queryWrapper)); - } finally { - queryWrapper.close(); - } + return convertToParts(listMPartitions(dbName, tblName, maxParts)); } }.run(false); } @@ -2242,32 +2323,31 @@ private boolean dropPartitionCommon(MPartition part) throws NoSuchObjectExceptio public List getPartitionsWithAuth(String dbName, String tblName, short max, String userName, List groupNames) throws MetaException, InvalidObjectException { - boolean success = false; - QueryWrapper queryWrapper = new QueryWrapper(); + boolean committed = false; try { openTransaction(); - List mparts = listMPartitions(dbName, tblName, max, queryWrapper); + List mparts = listMPartitions(dbName, tblName, max); List parts = new ArrayList<>(mparts.size()); - if (mparts != null && mparts.size()>0) { - for (MPartition mpart : mparts) { - MTable mtbl = mpart.getTable(); - Partition part = convertToPart(mpart); - parts.add(part); - - if ("TRUE".equalsIgnoreCase(mtbl.getParameters().get("PARTITION_LEVEL_PRIVILEGE"))) { - String partName = Warehouse.makePartName(this.convertToFieldSchemas(mtbl - .getPartitionKeys()), part.getValues()); - PrincipalPrivilegeSet partAuth = this.getPartitionPrivilegeSet(dbName, - tblName, partName, userName, groupNames); - part.setPrivileges(partAuth); - } + for (MPartition mpart : mparts) { + MTable mtbl = mpart.getTable(); + Partition part = convertToPart(mpart); + parts.add(part); + + if ("TRUE".equalsIgnoreCase(mtbl.getParameters().get("PARTITION_LEVEL_PRIVILEGE"))) { + String partName = Warehouse.makePartName(this.convertToFieldSchemas(mtbl + .getPartitionKeys()), part.getValues()); + PrincipalPrivilegeSet partAuth = this.getPartitionPrivilegeSet(dbName, + tblName, partName, userName, groupNames); + part.setPrivileges(partAuth); } } - success = commitTransaction(); + committed = commitTransaction(); return parts; } finally { - rollbackAndCleanup(success, queryWrapper); + if (!committed) { + rollbackTransaction(); + } } } @@ -2636,7 +2716,7 @@ private PartitionValuesResponse getDistinctValuesForPartitionsNoTxn(String dbNam * has types of String, and if resultsCol is null, the types are MPartition. */ private Collection getPartitionPsQueryResults(String dbName, String tableName, - List part_vals, short max_parts, String resultsCol, QueryWrapper queryWrapper) + List part_vals, short max_parts, String resultsCol) throws MetaException, NoSuchObjectException { dbName = normalizeIdentifier(dbName); tableName = normalizeIdentifier(tableName); @@ -2648,7 +2728,7 @@ private Collection getPartitionPsQueryResults(String dbName, String tableName, int numPartKeys = partCols.size(); if (part_vals.size() > numPartKeys) { throw new MetaException("Incorrect number of partition values." - + " numPartKeys=" + numPartKeys + ", part_val=" + part_vals.size()); + + " numPartKeys=" + numPartKeys + ", part_val=" + part_vals.size()); } partCols = partCols.subList(0, part_vals.size()); // Construct a pattern of the form: partKey=partVal/partKey2=partVal2/... @@ -2661,22 +2741,23 @@ private Collection getPartitionPsQueryResults(String dbName, String tableName, if (part_vals.size() < numPartKeys) { partNameMatcher += ".*"; } - Query query = queryWrapper.query = pm.newQuery(MPartition.class); - StringBuilder queryFilter = new StringBuilder("table.database.name == dbName"); - queryFilter.append(" && table.tableName == tableName"); - queryFilter.append(" && partitionName.matches(partialRegex)"); - query.setFilter(queryFilter.toString()); - query.declareParameters("java.lang.String dbName, " - + "java.lang.String tableName, java.lang.String partialRegex"); - if (max_parts >= 0) { - // User specified a row limit, set it on the Query - query.setRange(0, max_parts); - } - if (resultsCol != null && !resultsCol.isEmpty()) { - query.setResult(resultsCol); - } + try (QueryWrapper query = new QueryWrapper(pm.newQuery(MPartition.class))) { + StringBuilder queryFilter = new StringBuilder("table.database.name == dbName"); + queryFilter.append(" && table.tableName == tableName"); + queryFilter.append(" && partitionName.matches(partialRegex)"); + query.setFilter(queryFilter.toString()); + query.declareParameters("java.lang.String dbName, " + + "java.lang.String tableName, java.lang.String partialRegex"); + if (max_parts >= 0) { + // User specified a row limit, set it on the Query + query.setRange(0, max_parts); + } + if (resultsCol != null && !resultsCol.isEmpty()) { + query.setResult(resultsCol); + } - return (Collection) query.execute(dbName, tableName, partNameMatcher); + return new ArrayList<>((List) query.execute(dbName, tableName, partNameMatcher)); + } } @Override @@ -2684,14 +2765,13 @@ private Collection getPartitionPsQueryResults(String dbName, String tableName, List part_vals, short max_parts, String userName, List groupNames) throws MetaException, InvalidObjectException, NoSuchObjectException { List partitions = new ArrayList<>(); - boolean success = false; - QueryWrapper queryWrapper = new QueryWrapper(); + boolean committed = false; try { openTransaction(); LOG.debug("executing listPartitionNamesPsWithAuth"); Collection parts = getPartitionPsQueryResults(db_name, tbl_name, - part_vals, max_parts, null, queryWrapper); + part_vals, max_parts, null); MTable mtbl = getMTable(db_name, tbl_name); for (Object o : parts) { Partition part = convertToPart((MPartition) o); @@ -2706,9 +2786,11 @@ private Collection getPartitionPsQueryResults(String dbName, String tableName, } partitions.add(part); } - success = commitTransaction(); + committed = commitTransaction(); } finally { - rollbackAndCleanup(success, queryWrapper); + if(!committed) { + rollbackTransaction(); + } } return partitions; } @@ -2716,51 +2798,51 @@ private Collection getPartitionPsQueryResults(String dbName, String tableName, @Override public List listPartitionNamesPs(String dbName, String tableName, List part_vals, short max_parts) throws MetaException, NoSuchObjectException { - List partitionNames = new ArrayList<>(); - boolean success = false; - QueryWrapper queryWrapper = new QueryWrapper(); + List partitionNames = new ArrayList(); + boolean committed = false; try { openTransaction(); LOG.debug("Executing listPartitionNamesPs"); Collection names = getPartitionPsQueryResults(dbName, tableName, - part_vals, max_parts, "partitionName", queryWrapper); + part_vals, max_parts, "partitionName"); for (Object o : names) { partitionNames.add((String) o); } - success = commitTransaction(); + committed = commitTransaction(); } finally { - rollbackAndCleanup(success, queryWrapper); + if(!committed) { + rollbackTransaction(); + } } return partitionNames; } // TODO:pc implement max - private List listMPartitions(String dbName, String tableName, int max, QueryWrapper queryWrapper) { - boolean success = false; - List mparts = null; - try { - openTransaction(); - LOG.debug("Executing listMPartitions"); + private List listMPartitions(String dbName, String tableName, int max) { + LOG.debug("Executing listMPartitions"); + boolean committed = false; + openTransaction(); + try (QueryWrapper query = new QueryWrapper(pm.newQuery(MPartition.class, + "table.tableName == t1 && table.database.name == t2"))) { dbName = normalizeIdentifier(dbName); tableName = normalizeIdentifier(tableName); - Query query = queryWrapper.query = pm.newQuery(MPartition.class, "table.tableName == t1 && table.database.name == t2"); query.declareParameters("java.lang.String t1, java.lang.String t2"); query.setOrdering("partitionName ascending"); if (max > 0) { query.setRange(0, max); } - mparts = (List) query.execute(tableName, dbName); + List mparts = (List) query.execute(tableName, dbName); LOG.debug("Done executing query for listMPartitions"); pm.retrieveAll(mparts); - success = commitTransaction(); - LOG.debug("Done retrieving all objects for listMPartitions " + mparts); + committed = commitTransaction(); + LOG.debug("Done retrieving all objects for listMPartitions {}", mparts); + return new ArrayList<>(mparts); } finally { - if (!success) { + if (!committed) { rollbackTransaction(); } } - return mparts; } @Override @@ -3733,22 +3815,23 @@ private void removeUnusedColumnDescriptor(MColumnDescriptor oldCD) { return; } - boolean success = false; - QueryWrapper queryWrapper = new QueryWrapper(); + boolean committed = false; try { openTransaction(); LOG.debug("execute removeUnusedColumnDescriptor"); - List referencedSDs = listStorageDescriptorsWithCD(oldCD, 1, queryWrapper); + List referencedSDs = listStorageDescriptorsWithCD(oldCD, 1); //if no other SD references this CD, we can throw it out. if (referencedSDs != null && referencedSDs.isEmpty()) { pm.retrieve(oldCD); pm.deletePersistent(oldCD); } - success = commitTransaction(); + committed = commitTransaction(); LOG.debug("successfully deleted a CD in removeUnusedColumnDescriptor"); } finally { - rollbackAndCleanup(success, queryWrapper); + if(!committed) { + rollbackTransaction(); + } } } @@ -3773,36 +3856,27 @@ private void preDropStorageDescriptor(MStorageDescriptor msd) { /** * Get a list of storage descriptors that reference a particular Column Descriptor + * Should be always executed inside a transaction. * @param oldCD the column descriptor to get storage descriptors for * @param maxSDs the maximum number of SDs to return * @return a list of storage descriptors */ private List listStorageDescriptorsWithCD( - MColumnDescriptor oldCD, - long maxSDs, - QueryWrapper queryWrapper) { - boolean success = false; - List sds = null; - try { - openTransaction(); - LOG.debug("Executing listStorageDescriptorsWithCD"); - Query query = queryWrapper.query = pm.newQuery(MStorageDescriptor.class, "this.cd == inCD"); + MColumnDescriptor oldCD, + long maxSDs) { + LOG.debug("Executing listStorageDescriptorsWithCD"); + try (QueryWrapper query = new QueryWrapper(pm.newQuery(MStorageDescriptor.class, "this.cd == inCD"))) { query.declareParameters("MColumnDescriptor inCD"); if (maxSDs >= 0) { // User specified a row limit, set it on the Query query.setRange(0, maxSDs); } - sds = (List)query.execute(oldCD); + List sds = (List) query.execute(oldCD); LOG.debug("Done executing query for listStorageDescriptorsWithCD"); pm.retrieveAll(sds); - success = commitTransaction(); LOG.debug("Done retrieving all objects for listStorageDescriptorsWithCD"); - } finally { - if (!success) { - rollbackTransaction(); - } + return new ArrayList<>(sds); } - return sds; } private int getColumnIndexFromTableColumns(List cols, String col) { @@ -4507,8 +4581,7 @@ private MRoleMap getMSecurityUserRoleMap(String userName, PrincipalType principa @Override public boolean removeRole(String roleName) throws MetaException, NoSuchObjectException { - boolean success = false; - QueryWrapper queryWrapper = new QueryWrapper(); + boolean committed = false; try { openTransaction(); MRole mRol = getMRole(roleName); @@ -4517,60 +4590,56 @@ public boolean removeRole(String roleName) throws MetaException, // first remove all the membership, the membership that this role has // been granted List roleMap = listMRoleMembers(mRol.getRoleName()); - if (roleMap.size() > 0) { + if (!roleMap.isEmpty()) { pm.deletePersistentAll(roleMap); } List roleMember = listMSecurityPrincipalMembershipRole(mRol - .getRoleName(), PrincipalType.ROLE, queryWrapper); - if (roleMember.size() > 0) { + .getRoleName(), PrincipalType.ROLE); + if (!roleMember.isEmpty()) { pm.deletePersistentAll(roleMember); } - queryWrapper.close(); // then remove all the grants List userGrants = listPrincipalMGlobalGrants( mRol.getRoleName(), PrincipalType.ROLE); - if (userGrants.size() > 0) { + if (!userGrants.isEmpty()) { pm.deletePersistentAll(userGrants); } List dbGrants = listPrincipalAllDBGrant(mRol - .getRoleName(), PrincipalType.ROLE, queryWrapper); - if (dbGrants.size() > 0) { + .getRoleName(), PrincipalType.ROLE); + if (!dbGrants.isEmpty()) { pm.deletePersistentAll(dbGrants); } - queryWrapper.close(); List tabPartGrants = listPrincipalAllTableGrants( - mRol.getRoleName(), PrincipalType.ROLE, queryWrapper); - if (tabPartGrants.size() > 0) { + mRol.getRoleName(), PrincipalType.ROLE); + if (!tabPartGrants.isEmpty()) { pm.deletePersistentAll(tabPartGrants); } - queryWrapper.close(); List partGrants = listPrincipalAllPartitionGrants( - mRol.getRoleName(), PrincipalType.ROLE, queryWrapper); - if (partGrants.size() > 0) { + mRol.getRoleName(), PrincipalType.ROLE); + if (!partGrants.isEmpty()) { pm.deletePersistentAll(partGrants); } - queryWrapper.close(); List tblColumnGrants = listPrincipalAllTableColumnGrants( - mRol.getRoleName(), PrincipalType.ROLE, queryWrapper); - if (tblColumnGrants.size() > 0) { + mRol.getRoleName(), PrincipalType.ROLE); + if (!tblColumnGrants.isEmpty()) { pm.deletePersistentAll(tblColumnGrants); } - queryWrapper.close(); List partColumnGrants = listPrincipalAllPartitionColumnGrants( - mRol.getRoleName(), PrincipalType.ROLE, queryWrapper); - if (partColumnGrants.size() > 0) { + mRol.getRoleName(), PrincipalType.ROLE); + if (!partColumnGrants.isEmpty()) { pm.deletePersistentAll(partColumnGrants); } - queryWrapper.close(); // finally remove the role pm.deletePersistent(mRol); } - success = commitTransaction(); + committed = commitTransaction(); } finally { - rollbackAndCleanup(success, queryWrapper); + if (!committed) { + rollbackTransaction(); + } } - return success; + return committed; } /** @@ -4692,28 +4761,22 @@ private void getAllRoleAncestors(Set processedRoleNames, List return result; } + /** + * Should be always executed inside transaction. + */ @SuppressWarnings("unchecked") private List listMSecurityPrincipalMembershipRole(final String roleName, - final PrincipalType principalType, - QueryWrapper queryWrapper) { - boolean success = false; - List mRoleMemebership = null; - try { - LOG.debug("Executing listMSecurityPrincipalMembershipRole"); + final PrincipalType principalType) { + LOG.debug("Executing listMSecurityPrincipalMembershipRole"); - openTransaction(); - Query query = queryWrapper.query = pm.newQuery(MRoleMap.class, "principalName == t1 && principalType == t2"); + try(QueryWrapper query = new QueryWrapper(pm.newQuery(MRoleMap.class, + "principalName == t1 && principalType == t2"))) { query.declareParameters("java.lang.String t1, java.lang.String t2"); - mRoleMemebership = (List) query.execute(roleName, principalType.toString()); + List mRoleMemebership = (List) query.execute(roleName, principalType.toString()); pm.retrieveAll(mRoleMemebership); - success = commitTransaction(); LOG.debug("Done retrieving all objects for listMSecurityPrincipalMembershipRole"); - } finally { - if (!success) { - rollbackTransaction(); - } + return new ArrayList<>(mRoleMemebership); } - return mRoleMemebership; } @Override @@ -5753,22 +5816,12 @@ public boolean revokePrivileges(PrivilegeBag privileges, boolean grantOption) @Override public List listPrincipalDBGrantsAll( String principalName, PrincipalType principalType) { - QueryWrapper queryWrapper = new QueryWrapper(); - try { - return convertDB(listPrincipalAllDBGrant(principalName, principalType, queryWrapper)); - } finally { - queryWrapper.close(); - } + return convertDB(listPrincipalAllDBGrant(principalName, principalType)); } @Override public List listDBGrantsAll(String dbName) { - QueryWrapper queryWrapper = new QueryWrapper(); - try { - return convertDB(listDatabaseGrants(dbName, queryWrapper)); - } finally { - queryWrapper.close(); - } + return convertDB(listDatabaseGrants(dbName)); } private List convertDB(List privs) { @@ -5790,34 +5843,30 @@ public boolean revokePrivileges(PrivilegeBag privileges, boolean grantOption) @SuppressWarnings("unchecked") private List listPrincipalAllDBGrant(String principalName, - PrincipalType principalType, - QueryWrapper queryWrapper) { - boolean success = false; - Query query = null; - List mSecurityDBList = null; - try { - LOG.debug("Executing listPrincipalAllDBGrant"); - - openTransaction(); + PrincipalType principalType) { + LOG.debug("Executing listPrincipalAllDBGrant"); + boolean committed = false; + openTransaction(); + try (QueryWrapper query = new QueryWrapper(pm.newQuery(MDBPrivilege.class))) { + List mSecurityDBList; if (principalName != null && principalType != null) { - query = queryWrapper.query = pm.newQuery(MDBPrivilege.class, "principalName == t1 && principalType == t2"); + query.setFilter("principalName == t1 && principalType == t2"); query.declareParameters("java.lang.String t1, java.lang.String t2"); mSecurityDBList = - (List) query.execute(principalName, principalType.toString()); + (List) query.execute(principalName, principalType.toString()); } else { - query = queryWrapper.query = pm.newQuery(MDBPrivilege.class); mSecurityDBList = (List) query.execute(); } pm.retrieveAll(mSecurityDBList); - success = commitTransaction(); + committed = commitTransaction(); LOG.debug("Done retrieving all objects for listPrincipalAllDBGrant"); + return new ArrayList<>(mSecurityDBList); } finally { - if (!success) { + if (!committed) { rollbackTransaction(); } } - return mSecurityDBList; } @SuppressWarnings("unchecked") @@ -5969,20 +6018,19 @@ public void dropPartitionAllColumnGrantsNoTxn( } @SuppressWarnings("unchecked") - private List listDatabaseGrants(String dbName, QueryWrapper queryWrapper) { + private List listDatabaseGrants(String dbName) { dbName = normalizeIdentifier(dbName); boolean success = false; - try { - LOG.debug("Executing listDatabaseGrants"); + openTransaction(); + try (QueryWrapper query = new QueryWrapper(pm.newQuery(MDBPrivilege.class, + "database.name == t1"))) { - openTransaction(); - Query query = queryWrapper.query = pm.newQuery(MDBPrivilege.class, "database.name == t1"); query.declareParameters("java.lang.String t1"); List mSecurityDBList = (List) query.executeWithArray(dbName); pm.retrieveAll(mSecurityDBList); success = commitTransaction(); LOG.debug("Done retrieving all objects for listDatabaseGrants"); - return mSecurityDBList; + return new ArrayList<>(mSecurityDBList); } finally { if (!success) { rollbackTransaction(); @@ -6379,30 +6427,24 @@ private void dropPartitionGrantsNoTxn(String dbName, String tableName, List listPrincipalAllTableGrants( - String principalName, PrincipalType principalType, QueryWrapper queryWrapper) { - boolean success = false; - List mSecurityTabPartList = null; - try { - LOG.debug("Executing listPrincipalAllTableGrants"); + String principalName, PrincipalType principalType) { + LOG.debug("Executing listPrincipalAllTableGrants"); - openTransaction(); - Query query = queryWrapper.query = pm.newQuery(MTablePrivilege.class, - "principalName == t1 && principalType == t2"); + try (QueryWrapper query = new QueryWrapper(pm.newQuery(MTablePrivilege.class, + "principalName == t1 && principalType == t2"))) { query.declareParameters("java.lang.String t1, java.lang.String t2"); - mSecurityTabPartList = (List) query.execute( - principalName, principalType.toString()); + List mSecurityTabPartList = + (List) query.execute(principalName, principalType.toString()); pm.retrieveAll(mSecurityTabPartList); - success = commitTransaction(); LOG.debug("Done retrieving all objects for listPrincipalAllTableGrants"); - } finally { - if (!success) { - rollbackTransaction(); - } + return new ArrayList<>(mSecurityTabPartList); } - return mSecurityTabPartList; } @Override @@ -6478,27 +6520,22 @@ private void dropPartitionGrantsNoTxn(String dbName, String tableName, List listPrincipalAllPartitionGrants(String principalName, - PrincipalType principalType, QueryWrapper queryWrapper) { - boolean success = false; - List mSecurityTabPartList = null; - try { - openTransaction(); - LOG.debug("Executing listPrincipalAllPartitionGrants"); - Query query = queryWrapper.query = pm.newQuery(MPartitionPrivilege.class, "principalName == t1 && principalType == t2"); + PrincipalType principalType) { + LOG.debug("Executing listPrincipalAllPartitionGrants"); + try (QueryWrapper query = new QueryWrapper(pm.newQuery(MPartitionPrivilege.class, + "principalName == t1 && principalType == t2"))) { query.declareParameters("java.lang.String t1, java.lang.String t2"); - mSecurityTabPartList = - (List) query.execute(principalName, principalType.toString()); + List mSecurityTabPartList = + (List) query.execute(principalName, principalType.toString()); pm.retrieveAll(mSecurityTabPartList); - success = commitTransaction(); LOG.debug("Done retrieving all objects for listPrincipalAllPartitionGrants"); - } finally { - if (!success) { - rollbackTransaction(); - } + return new ArrayList<>(mSecurityTabPartList); } - return mSecurityTabPartList; } @Override @@ -6577,31 +6614,24 @@ private void dropPartitionGrantsNoTxn(String dbName, String tableName, List listPrincipalAllTableColumnGrants(String principalName, - PrincipalType principalType, QueryWrapper queryWrapper) { - boolean success = false; - - List mSecurityColumnList = null; - try { - LOG.debug("Executing listPrincipalAllTableColumnGrants"); + PrincipalType principalType) { + LOG.debug("Executing listPrincipalAllTableColumnGrants"); - openTransaction(); - Query query = queryWrapper.query = - pm.newQuery(MTableColumnPrivilege.class, "principalName == t1 && principalType == t2"); + try (QueryWrapper query = new QueryWrapper(pm.newQuery(MTableColumnPrivilege.class, + "principalName == t1 && principalType == t2"))) { query.declareParameters("java.lang.String t1, java.lang.String t2"); - mSecurityColumnList = - (List) query.execute(principalName, principalType.toString()); + List mSecurityColumnList = + (List) query.execute(principalName, principalType.toString()); pm.retrieveAll(mSecurityColumnList); - success = commitTransaction(); LOG.debug("Done retrieving all objects for listPrincipalAllTableColumnGrants"); - } finally { - if (!success) { - rollbackTransaction(); - } + return new ArrayList<>(mSecurityColumnList); } - return mSecurityColumnList; } @Override @@ -6681,30 +6711,24 @@ private void dropPartitionGrantsNoTxn(String dbName, String tableName, List listPrincipalAllPartitionColumnGrants( - String principalName, PrincipalType principalType, QueryWrapper queryWrapper) { - boolean success = false; - List mSecurityColumnList = null; - try { - LOG.debug("Executing listPrincipalAllTableColumnGrants"); + String principalName, PrincipalType principalType) { + LOG.debug("Executing listPrincipalAllTableColumnGrants"); - openTransaction(); - Query query = queryWrapper.query = - pm.newQuery(MPartitionColumnPrivilege.class, "principalName == t1 && principalType == t2"); + try (QueryWrapper query = new QueryWrapper(pm.newQuery(MPartitionColumnPrivilege.class, + "principalName == t1 && principalType == t2"))) { query.declareParameters("java.lang.String t1, java.lang.String t2"); - mSecurityColumnList = - (List) query.execute(principalName, principalType.toString()); + List mSecurityColumnList = + (List) query.execute(principalName, principalType.toString()); pm.retrieveAll(mSecurityColumnList); - success = commitTransaction(); LOG.debug("Done retrieving all objects for listPrincipalAllTableColumnGrants"); - } finally { - if (!success) { - rollbackTransaction(); - } + return new ArrayList<>(mSecurityColumnList); } - return mSecurityColumnList; } @Override @@ -6786,20 +6810,13 @@ private String getPartitionStr(Table tbl, Map partName) throws In * is used by HiveMetaTool. This API **shouldn't** be exposed via Thrift. * */ - public Collection executeJDOQLSelect(String queryStr, QueryWrapper queryWrapper) { + public Collection executeJDOQLSelect(String queryStr) { boolean committed = false; - Collection result = null; - try { - openTransaction(); - Query query = queryWrapper.query = pm.newQuery(queryStr); - result = ((Collection) query.execute()); + openTransaction(); + try (QueryWrapper query = new QueryWrapper(pm.newQuery(queryStr))) { + Collection result = ((Collection) query.execute()); committed = commitTransaction(); - - if (committed) { - return result; - } else { - return null; - } + return committed ? result : null; } finally { if (!committed) { rollbackTransaction(); @@ -7270,20 +7287,15 @@ private void writeMTableColumnStatistics(Table table, MTableColumnStatistics mSt String dbName = mStatsObj.getDbName(); String tableName = mStatsObj.getTableName(); String colName = mStatsObj.getColName(); - QueryWrapper queryWrapper = new QueryWrapper(); - try { - LOG.info("Updating table level column statistics for db=" + dbName + " tableName=" + tableName - + " colName=" + colName); - validateTableCols(table, Lists.newArrayList(colName)); + LOG.info("Updating table level column statistics for db=" + dbName + " tableName=" + tableName + + " colName=" + colName); + validateTableCols(table, Lists.newArrayList(colName)); - if (oldStats != null) { - StatObjectConverter.setFieldsIntoOldStats(mStatsObj, oldStats); - } else { - pm.makePersistent(mStatsObj); - } - } finally { - queryWrapper.close(); + if (oldStats != null) { + StatObjectConverter.setFieldsIntoOldStats(mStatsObj, oldStats); + } else { + pm.makePersistent(mStatsObj); } } @@ -7311,15 +7323,10 @@ private void writeMPartitionColumnStatistics(Table table, Partition partition, LOG.warn("Column " + colName + " for which stats gathering is requested doesn't exist."); } - QueryWrapper queryWrapper = new QueryWrapper(); - try { - if (oldStats != null) { - StatObjectConverter.setFieldsIntoOldStats(mStatsObj, oldStats); - } else { - pm.makePersistent(mStatsObj); - } - } finally { - queryWrapper.close(); + if (oldStats != null) { + StatObjectConverter.setFieldsIntoOldStats(mStatsObj, oldStats); + } else { + pm.makePersistent(mStatsObj); } } @@ -7334,16 +7341,14 @@ private void writeMPartitionColumnStatistics(Table table, Partition partition, */ private Map getPartitionColStats(Table table, List colNames) throws NoSuchObjectException, MetaException { - Map statsMap = Maps.newHashMap(); - QueryWrapper queryWrapper = new QueryWrapper(); - try { - List stats = getMTableColumnStatistics(table, - colNames, queryWrapper); - for(MTableColumnStatistics cStat : stats) { - statsMap.put(cStat.getColName(), cStat); - } - } finally { - queryWrapper.close(); + List stats = getMTableColumnStatistics(table, + colNames); + if (stats == null) { + return Collections.emptyMap(); + } + Map statsMap = new HashMap<>(stats.size()); + for(MTableColumnStatistics cStat : stats) { + statsMap.put(cStat.getColName(), cStat); } return statsMap; } @@ -7405,16 +7410,14 @@ public boolean updateTableColumnStatistics(ColumnStatistics colStats) */ private Map getPartitionColStats(Table table, String partitionName, List colNames) throws NoSuchObjectException, MetaException { - Map statsMap = Maps.newHashMap(); - QueryWrapper queryWrapper = new QueryWrapper(); - try { - List stats = getMPartitionColumnStatistics(table, - Lists.newArrayList(partitionName), colNames, queryWrapper); - for(MPartitionColumnStatistics cStat : stats) { - statsMap.put(cStat.getColName(), cStat); - } - } finally { - queryWrapper.close(); + List stats = getMPartitionColumnStatistics(table, + Lists.newArrayList(partitionName), colNames); + if (stats.isEmpty()) { + return Collections.emptyMap(); + } + Map statsMap = new HashMap<>(stats.size()); + for(MPartitionColumnStatistics cStat : stats) { + statsMap.put(cStat.getColName(), cStat); } return statsMap; } @@ -7464,40 +7467,39 @@ public boolean updatePartitionColumnStatistics(ColumnStatistics colStats, List getMTableColumnStatistics(Table table, List colNames, QueryWrapper queryWrapper) + private List getMTableColumnStatistics(Table table, List colNames) throws MetaException { if (colNames == null || colNames.isEmpty()) { return null; } - boolean committed = false; - try { - openTransaction(); + validateTableCols(table, colNames); + String filter = "tableName == t1 && dbName == t2 && ("; + String paramStr = "java.lang.String t1, java.lang.String t2"; + Object[] params = new Object[colNames.size() + 2]; + params[0] = table.getTableName(); + params[1] = table.getDbName(); + for (int i = 0; i < colNames.size(); ++i) { + filter += ((i == 0) ? "" : " || ") + "colName == c" + i; + paramStr += ", java.lang.String c" + i; + params[i + 2] = colNames.get(i); + } + filter += ")"; + boolean committed = false; + openTransaction(); + try (QueryWrapper query = new QueryWrapper(pm.newQuery(MTableColumnStatistics.class))) { List result = null; - validateTableCols(table, colNames); - Query query = queryWrapper.query = pm.newQuery(MTableColumnStatistics.class); - String filter = "tableName == t1 && dbName == t2 && ("; - String paramStr = "java.lang.String t1, java.lang.String t2"; - Object[] params = new Object[colNames.size() + 2]; - params[0] = table.getTableName(); - params[1] = table.getDbName(); - for (int i = 0; i < colNames.size(); ++i) { - filter += ((i == 0) ? "" : " || ") + "colName == c" + i; - paramStr += ", java.lang.String c" + i; - params[i + 2] = colNames.get(i); - } - filter += ")"; query.setFilter(filter); query.declareParameters(paramStr); result = (List) query.executeWithArray(params); pm.retrieveAll(result); if (result.size() > colNames.size()) { throw new MetaException("Unexpected " + result.size() + " statistics for " - + colNames.size() + " columns"); + + colNames.size() + " columns"); } committed = commitTransaction(); - return result; + return new ArrayList<>(result); } catch (Exception ex) { LOG.error("Error retrieving statistics via jdo", ex); if (ex instanceof MetaException) { @@ -7548,10 +7550,7 @@ protected ColumnStatistics getSqlResult(GetHelper ctx) throws @Override protected ColumnStatistics getJdoResult( GetHelper ctx) throws MetaException { - QueryWrapper queryWrapper = new QueryWrapper(); - - try { - List mStats = getMTableColumnStatistics(getTable(), colNames, queryWrapper); + List mStats = getMTableColumnStatistics(getTable(), colNames); if (mStats.isEmpty()) return null; // LastAnalyzed is stored per column, but thrift object has it per multiple columns. // Luckily, nobody actually uses it, so we will set to lowest value of all columns for now. @@ -7565,9 +7564,6 @@ protected ColumnStatistics getJdoResult( Deadline.checkTimeout(); } return new ColumnStatistics(desc, statObjs); - } finally { - queryWrapper.close(); - } } }.run(true); } @@ -7592,10 +7588,8 @@ protected ColumnStatistics getJdoResult( @Override protected List getJdoResult( GetHelper> ctx) throws MetaException, NoSuchObjectException { - QueryWrapper queryWrapper = new QueryWrapper(); - try { List mStats = - getMPartitionColumnStatistics(getTable(), partNames, colNames, queryWrapper); + getMPartitionColumnStatistics(getTable(), partNames, colNames); List result = new ArrayList<>( Math.min(mStats.size(), partNames.size())); String lastPartName = null; @@ -7620,9 +7614,6 @@ protected ColumnStatistics getJdoResult( Deadline.checkTimeout(); } return result; - } finally { - queryWrapper.close(); - } } }.run(true); } @@ -7691,49 +7682,50 @@ public void flushCache() { } private List getMPartitionColumnStatistics( - Table table, List partNames, List colNames, QueryWrapper queryWrapper) + Table table, List partNames, List colNames) throws NoSuchObjectException, MetaException { + // We are not going to verify SD for each partition. Just verify for the table. + // ToDo: we need verify the partition column instead + try { + validateTableCols(table, colNames); + } catch (MetaException me) { + LOG.warn("The table does not have the same column definition as its partition."); + } + + String paramStr = "java.lang.String t1, java.lang.String t2"; + String filter = "tableName == t1 && dbName == t2 && ("; + Object[] params = new Object[colNames.size() + partNames.size() + 2]; + int i = 0; + params[i++] = table.getTableName(); + params[i++] = table.getDbName(); + int firstI = i; + for (String s : partNames) { + filter += ((i == firstI) ? "" : " || ") + "partitionName == p" + i; + paramStr += ", java.lang.String p" + i; + params[i++] = s; + } + filter += ") && ("; + firstI = i; + for (String s : colNames) { + filter += ((i == firstI) ? "" : " || ") + "colName == c" + i; + paramStr += ", java.lang.String c" + i; + params[i++] = s; + } + filter += ")"; + boolean committed = false; + openTransaction(); - try { - openTransaction(); - // We are not going to verify SD for each partition. Just verify for the table. - // ToDo: we need verify the partition column instead - try { - validateTableCols(table, colNames); - } catch (MetaException me) { - LOG.warn("The table does not have the same column definition as its partition."); - } - Query query = queryWrapper.query = pm.newQuery(MPartitionColumnStatistics.class); - String paramStr = "java.lang.String t1, java.lang.String t2"; - String filter = "tableName == t1 && dbName == t2 && ("; - Object[] params = new Object[colNames.size() + partNames.size() + 2]; - int i = 0; - params[i++] = table.getTableName(); - params[i++] = table.getDbName(); - int firstI = i; - for (String s : partNames) { - filter += ((i == firstI) ? "" : " || ") + "partitionName == p" + i; - paramStr += ", java.lang.String p" + i; - params[i++] = s; - } - filter += ") && ("; - firstI = i; - for (String s : colNames) { - filter += ((i == firstI) ? "" : " || ") + "colName == c" + i; - paramStr += ", java.lang.String c" + i; - params[i++] = s; - } - filter += ")"; + try(QueryWrapper query = new QueryWrapper(pm.newQuery(MPartitionColumnStatistics.class))) { query.setFilter(filter); query.declareParameters(paramStr); query.setOrdering("partitionName ascending"); @SuppressWarnings("unchecked") List result = - (List) query.executeWithArray(params); + (List) query.executeWithArray(params); pm.retrieveAll(result); committed = commitTransaction(); - return result; + return new ArrayList<>(result); } catch (Exception ex) { LOG.error("Error retrieving statistics via jdo", ex); if (ex instanceof MetaException) { @@ -7743,7 +7735,7 @@ public void flushCache() { } finally { if (!committed) { rollbackTransaction(); - return Lists.newArrayList(); + return Collections.emptyList(); } } } -- 2.14.1