You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2020/09/14 16:51:34 UTC
[GitHub] [hive] pvargacl opened a new pull request #1498: HIVE-24162: Query based compaction looses bloom filter
pvargacl opened a new pull request #1498:
URL: https://github.com/apache/hive/pull/1498
### What changes were proposed in this pull request?
Keep the orc.bloom.filter during Query based compaction.
### Why are the changes needed?
Fix the bug
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Test added
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvargacl commented on pull request #1498: HIVE-24162: Query based compaction looses bloom filter
Posted by GitBox <gi...@apache.org>.
pvargacl commented on pull request #1498:
URL: https://github.com/apache/hive/pull/1498#issuecomment-692497872
@klcopp @laszlopinter86 could you take a look please?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvargacl commented on a change in pull request #1498: HIVE-24162: Query based compaction looses bloom filter
Posted by GitBox <gi...@apache.org>.
pvargacl commented on a change in pull request #1498:
URL: https://github.com/apache/hive/pull/1498#discussion_r489203951
##########
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:
Well the tblproperties will not get overwritten in the original table, so only the properties matter, that influence the compacted file itself. I think orc properties should cover those.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] klcopp merged pull request #1498: HIVE-24162: Query based compaction looses bloom filter
Posted by GitBox <gi...@apache.org>.
klcopp merged pull request #1498:
URL: https://github.com/apache/hive/pull/1498
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] klcopp commented on a change in pull request #1498: HIVE-24162: Query based compaction looses bloom filter
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] klcopp commented on a change in pull request #1498: HIVE-24162: Query based compaction looses bloom filter
Posted by GitBox <gi...@apache.org>.
klcopp commented on a change in pull request #1498:
URL: https://github.com/apache/hive/pull/1498#discussion_r489232187
##########
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:
Makes sense!
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org