Skip to content

Commit c18d0df

Browse files
kasakriszkasakrisz
authored
Enable using column identifiers with special characters when deleting table column statistics. (#6149)
1 parent 770e70c commit c18d0df

File tree

2 files changed

+57
-21
lines changed

2 files changed

+57
-21
lines changed

‎standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java‎

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@
133133
*/
134134
class MetaStoreDirectSql {
135135
private static final int NO_BATCHING = -1, DETECT_BATCHING = 0;
136+
private static final Set<String> ALLOWED_TABLES_TO_LOCK = Set.of("NOTIFICATION_SEQUENCE");
136137

137138
private static final Logger LOG = LoggerFactory.getLogger(MetaStoreDirectSql.class);
138139
private final PersistenceManager pm;
@@ -3203,6 +3204,11 @@ private void getStatsTableListResult(
32033204
}
32043205

32053206
public void lockDbTable(String tableName) throws MetaException {
3207+
// Only certain tables are allowed to be locked, and the API should restrict them.
3208+
if (!ALLOWED_TABLES_TO_LOCK.contains(tableName)) {
3209+
throw new MetaException("Error while locking table " + tableName);
3210+
}
3211+
32063212
String lockCommand = "lock table \"" + tableName + "\" in exclusive mode";
32073213
try {
32083214
executeNoResult(lockCommand);
@@ -3243,19 +3249,26 @@ public void deleteColumnStatsState(long tbl_id) throws MetaException {
32433249
}
32443250

32453251
public boolean deleteTableColumnStatistics(long tableId, List<String> colNames, String engine) {
3246-
String deleteSql = "delete from " + TAB_COL_STATS + " where \"TBL_ID\" = " + tableId;
3252+
String deleteSql = "delete from " + TAB_COL_STATS + " where \"TBL_ID\" = ?";
3253+
List<Object> params = new ArrayList<>(colNames == null ? 2 : colNames.size() + 2);
3254+
params.add(tableId);
3255+
32473256
if (colNames != null && !colNames.isEmpty()) {
3248-
deleteSql += " and \"COLUMN_NAME\" in (" + colNames.stream().map(col -> "'" + col + "'").collect(Collectors.joining(",")) + ")";
3257+
deleteSql += " and \"COLUMN_NAME\" in (" + makeParams(colNames.size()) + ")";
3258+
params.addAll(colNames);
32493259
}
3260+
32503261
if (engine != null) {
3251-
deleteSql += " and \"ENGINE\" = '" + engine + "'";
3262+
deleteSql += " and \"ENGINE\" = ?";
3263+
params.add(engine);
32523264
}
3253-
try {
3254-
executeNoResult(deleteSql);
3255-
} catch (SQLException e) {
3256-
LOG.warn("Error removing table column stats. ", e);
3265+
3266+
try (QueryWrapper queryParams = new QueryWrapper(pm.newQuery("javax.jdo.query.SQL", deleteSql))) {
3267+
executeWithArray(queryParams.getInnerQuery(), params.toArray(), deleteSql);
3268+
} catch (MetaException e) {
32573269
return false;
32583270
}
3271+
32593272
return true;
32603273
}
32613274

@@ -3269,17 +3282,20 @@ public List<Void> run(List<String> input) throws Exception {
32693282
input, Collections.emptyList(), -1);
32703283
if (!partitionIds.isEmpty()) {
32713284
String deleteSql = "delete from " + PART_COL_STATS + " where \"PART_ID\" in ( " + getIdListForIn(partitionIds) + ")";
3285+
List<Object> params = new ArrayList<>(colNames == null ? 1 : colNames.size() + 1);
3286+
32723287
if (colNames != null && !colNames.isEmpty()) {
3273-
deleteSql += " and \"COLUMN_NAME\" in (" + colNames.stream().map(col -> "'" + col + "'").collect(Collectors.joining(",")) + ")";
3288+
deleteSql += " and \"COLUMN_NAME\" in (" + makeParams(colNames.size()) + ")";
3289+
params.addAll(colNames);
32743290
}
3291+
32753292
if (engine != null) {
3276-
deleteSql += " and \"ENGINE\" = '" + engine + "'";
3293+
deleteSql += " and \"ENGINE\" = ?";
3294+
params.add(engine);
32773295
}
3278-
try {
3279-
executeNoResult(deleteSql);
3280-
} catch (SQLException e) {
3281-
LOG.warn("Error removing partition column stats. ", e);
3282-
throw new MetaException("Error removing partition column stats: " + e.getMessage());
3296+
3297+
try (QueryWrapper queryParams = new QueryWrapper(pm.newQuery("javax.jdo.query.SQL", deleteSql))) {
3298+
executeWithArray(queryParams.getInnerQuery(), params.toArray(), deleteSql);
32833299
}
32843300
}
32853301
return null;

‎standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -919,42 +919,48 @@ public void testTableStatisticsOps() throws Exception {
919919
List<ColumnStatistics> tabColStats;
920920
try (AutoCloseable c = deadline()) {
921921
tabColStats = objectStore.getTableColumnStatistics(DEFAULT_CATALOG_NAME, DB1, TABLE1,
922-
Arrays.asList("test_col1", "test_col2"));
922+
Arrays.asList("test_col1", "test_col' 2"));
923923
}
924924
Assert.assertEquals(0, tabColStats.size());
925925

926926
ColumnStatisticsDesc statsDesc = new ColumnStatisticsDesc(true, DB1, TABLE1);
927927
ColumnStatisticsObj statsObj1 = new ColumnStatisticsObj("test_col1", "int",
928928
new ColumnStatisticsData(ColumnStatisticsData._Fields.DECIMAL_STATS, new DecimalColumnStatsData(100, 1000)));
929-
ColumnStatisticsObj statsObj2 = new ColumnStatisticsObj("test_col2", "int",
929+
ColumnStatisticsObj statsObj2 = new ColumnStatisticsObj("test_col' 2", "int",
930930
new ColumnStatisticsData(ColumnStatisticsData._Fields.DECIMAL_STATS, new DecimalColumnStatsData(200, 2000)));
931931
ColumnStatistics colStats = new ColumnStatistics(statsDesc, Arrays.asList(statsObj1, statsObj2));
932932
colStats.setEngine(ENGINE);
933933
objectStore.updateTableColumnStatistics(colStats, null, 0);
934934

935935
try (AutoCloseable c = deadline()) {
936936
tabColStats = objectStore.getTableColumnStatistics(DEFAULT_CATALOG_NAME, DB1, TABLE1,
937-
Arrays.asList("test_col1", "test_col2"));
937+
Arrays.asList("test_col1", "test_col' 2"));
938938
}
939939
Assert.assertEquals(1, tabColStats.size());
940940
Assert.assertEquals(2, tabColStats.get(0).getStatsObjSize());
941941

942942
objectStore.deleteTableColumnStatistics(DEFAULT_CATALOG_NAME, DB1, TABLE1, "test_col1", ENGINE);
943943
try (AutoCloseable c = deadline()) {
944944
tabColStats = objectStore.getTableColumnStatistics(DEFAULT_CATALOG_NAME, DB1, TABLE1,
945-
Arrays.asList("test_col1", "test_col2"));
945+
Arrays.asList("test_col1", "test_col' 2"));
946946
}
947947
Assert.assertEquals(1, tabColStats.size());
948948
Assert.assertEquals(1, tabColStats.get(0).getStatsObjSize());
949949

950-
objectStore.deleteTableColumnStatistics(DEFAULT_CATALOG_NAME, DB1, TABLE1, "test_col2", ENGINE);
950+
objectStore.deleteTableColumnStatistics(DEFAULT_CATALOG_NAME, DB1, TABLE1, "test_col' 2", ENGINE);
951951
try (AutoCloseable c = deadline()) {
952952
tabColStats = objectStore.getTableColumnStatistics(DEFAULT_CATALOG_NAME, DB1, TABLE1,
953-
Arrays.asList("test_col1", "test_col2"));
953+
Arrays.asList("test_col1", "test_col' 2"));
954954
}
955955
Assert.assertEquals(0, tabColStats.size());
956956
}
957957

958+
@Test
959+
public void testDeleteTableColumnStatisticsWhenEngineHasSpecialCharacter() throws Exception {
960+
createPartitionedTable(true, true);
961+
objectStore.deleteTableColumnStatistics(DEFAULT_CATALOG_NAME, DB1, TABLE1, "test_col1", "special '");
962+
}
963+
958964
@Test
959965
public void testPartitionStatisticsOps() throws Exception {
960966
createPartitionedTable(true, true);
@@ -1006,6 +1012,14 @@ public void testPartitionStatisticsOps() throws Exception {
10061012
Assert.assertEquals(0, stat.size());
10071013
}
10081014

1015+
@Test
1016+
public void testDeletePartitionColumnStatisticsWhenEngineHasSpecialCharacter() throws Exception {
1017+
createPartitionedTable(true, true);
1018+
objectStore.deletePartitionColumnStatistics(DEFAULT_CATALOG_NAME, DB1, TABLE1,
1019+
"test_part_col=a2", List.of("a2"), null, "special '");
1020+
}
1021+
1022+
10091023
@Test
10101024
public void testAggrStatsUseDB() throws Exception {
10111025
Configuration conf2 = MetastoreConf.newMetastoreConf(conf);
@@ -1051,7 +1065,7 @@ private void createPartitionedTable(boolean withPrivileges, boolean withStatisti
10511065
.setDbName(DB1)
10521066
.setTableName(TABLE1)
10531067
.addCol("test_col1", "int")
1054-
.addCol("test_col2", "int")
1068+
.addCol("test_col' 2", "int")
10551069
.addPartCol("test_part_col", "int")
10561070
.addCol("test_bucket_col", "int", "test bucket col comment")
10571071
.addCol("test_skewed_col", "int", "test skewed col comment")
@@ -1239,6 +1253,12 @@ protected Database getJdoResult(ObjectStore.GetHelper<Database> ctx) throws Meta
12391253
Assert.assertEquals(1, directSqlErrors.getCount());
12401254
}
12411255

1256+
@Test(expected = MetaException.class)
1257+
public void testLockDbTableThrowsExceptionWhenTableIsNotAllowedToLock() throws Exception {
1258+
MetaStoreDirectSql metaStoreDirectSql = new MetaStoreDirectSql(objectStore.getPersistenceManager(), conf, null);
1259+
metaStoreDirectSql.lockDbTable("TBLS");
1260+
}
1261+
12421262
@Deprecated
12431263
private static void dropAllStoreObjects(RawStore store)
12441264
throws MetaException, InvalidObjectException, InvalidInputException {

0 commit comments

Comments
 (0)