klcopp commented on a change in pull request #1498:
URL: https://github.com/apache/hive/pull/1498#discussion_r488761512



##########
File path: 
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactionQueryBuilder.java
##########
@@ -543,18 +543,26 @@ private void addTblProperties(StringBuilder query, int 
bucketingVersion) {
     if (crud && minor && isBucketed) {
       tblProperties.put("bucketing_version", String.valueOf(bucketingVersion));
     }
-    if (insertOnly && sourceTab != null) { // to avoid NPEs, skip this part if 
sourceTab is null
-      // Exclude all standard table properties.
-      Set<String> excludes = getHiveMetastoreConstants();
-      excludes.addAll(StatsSetupConst.TABLE_PARAMS_STATS_KEYS);
-      for (Map.Entry<String, String> e : sourceTab.getParameters().entrySet()) 
{
-        if (e.getValue() == null) {
-          continue;
+    if (sourceTab != null) { // to avoid NPEs, skip this part if sourceTab is 
null
+      if (insertOnly) {
+        // Exclude all standard table properties.
+        Set<String> excludes = getHiveMetastoreConstants();
+        excludes.addAll(StatsSetupConst.TABLE_PARAMS_STATS_KEYS);
+        for (Map.Entry<String, String> e : 
sourceTab.getParameters().entrySet()) {
+          if (e.getValue() == null) {
+            continue;
+          }
+          if (excludes.contains(e.getKey())) {
+            continue;
+          }
+          tblProperties.put(e.getKey(), 
HiveStringUtils.escapeHiveCommand(e.getValue()));
         }
-        if (excludes.contains(e.getKey())) {
-          continue;
+      } else if (crud) {

Review comment:
       Nit: Just an "else" works

##########
File path: 
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorOnTezTest.java
##########
@@ -144,11 +149,11 @@ void createMmTable(String tblName, boolean isPartitioned, 
boolean isBucketed, St
 
     void createMmTable(String dbName, String tblName, boolean isPartitioned, 
boolean isBucketed, String fileFormat)
         throws Exception {
-      createTable(dbName, tblName, isPartitioned, isBucketed, true, 
fileFormat);
+      createTable(dbName, tblName, isPartitioned, isBucketed, true, 
fileFormat, false);
     }
 
     private void createTable(String dbName, String tblName, boolean 
isPartitioned, boolean isBucketed,
-        boolean insertOnly, String fileFormat) throws Exception {
+        boolean insertOnly, String fileFormat, boolean addBloomFilter) throws 
Exception {

Review comment:
       Nit: maybe add a param for additional tblproperties instead of a 
boolean, for easier reading and more flexibility

##########
File path: 
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactionQueryBuilder.java
##########
@@ -543,18 +543,26 @@ private void addTblProperties(StringBuilder query, int 
bucketingVersion) {
     if (crud && minor && isBucketed) {
       tblProperties.put("bucketing_version", String.valueOf(bucketingVersion));
     }
-    if (insertOnly && sourceTab != null) { // to avoid NPEs, skip this part if 
sourceTab is null
-      // Exclude all standard table properties.
-      Set<String> excludes = getHiveMetastoreConstants();
-      excludes.addAll(StatsSetupConst.TABLE_PARAMS_STATS_KEYS);
-      for (Map.Entry<String, String> e : sourceTab.getParameters().entrySet()) 
{
-        if (e.getValue() == null) {
-          continue;
+    if (sourceTab != null) { // to avoid NPEs, skip this part if sourceTab is 
null
+      if (insertOnly) {
+        // Exclude all standard table properties.
+        Set<String> excludes = getHiveMetastoreConstants();
+        excludes.addAll(StatsSetupConst.TABLE_PARAMS_STATS_KEYS);
+        for (Map.Entry<String, String> e : 
sourceTab.getParameters().entrySet()) {
+          if (e.getValue() == null) {
+            continue;
+          }
+          if (excludes.contains(e.getKey())) {
+            continue;
+          }
+          tblProperties.put(e.getKey(), 
HiveStringUtils.escapeHiveCommand(e.getValue()));
         }
-        if (excludes.contains(e.getKey())) {
-          continue;
+      } else if (crud) {
+        for (Map.Entry<String, String> e : 
sourceTab.getParameters().entrySet()) {
+          if (e.getKey().startsWith("orc.")) {

Review comment:
       Just thinking, are there more properties that need to be included in the 
compacted file, besides orc properties? Maybe not...




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

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