You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2020/01/27 21:49:31 UTC

[impala] branch master updated (6ac5152 -> 567b3cd)

This is an automated email from the ASF dual-hosted git repository.

tarmstrong pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git.


    from 6ac5152  IMPALA-9291: MemTracker's TryConsume() should behave consistent with Consume()/Release()
     new 641c406  IMPALA-9242: Bump CDH_BUILD_NUMBER to 1814051
     new 567b3cd  IMPALA-9311: Store SQLPrimaryKeys in canonical order.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 bin/impala-config.sh                                   |  2 +-
 .../main/java/org/apache/impala/catalog/HdfsTable.java |  3 +++
 .../impala/catalog/local/DirectMetaProvider.java       |  3 +++
 .../java/org/apache/impala/util/MetaStoreUtil.java     | 18 ++++++++++++++++++
 .../java/org/apache/impala/catalog/CatalogTest.java    |  6 ++----
 .../apache/impala/catalog/PartialCatalogInfoTest.java  |  6 ++----
 .../apache/impala/catalog/local/LocalCatalogTest.java  |  6 ++----
 .../queries/QueryTest/show-create-table.test           |  2 +-
 8 files changed, 32 insertions(+), 14 deletions(-)


[impala] 02/02: IMPALA-9311: Store SQLPrimaryKeys in canonical order.

Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 567b3cd04c7ecd43282127c47b21f177ecb9a2b5
Author: Anurag Mantripragada <an...@cloudera.com>
AuthorDate: Thu Jan 23 22:04:50 2020 -0800

    IMPALA-9311: Store SQLPrimaryKeys in canonical order.
    
    HMS seems to be returning SQLPrimaryKeys in inconsistent orders.
    This makes some of the primary keys tests flaky. This change sorts
    the list of primary keys and stores them in canonical order within
    Impala.
    
    Testing:
    - Modified the tests that were relying on HMS to return same order
      every time.
    - Ran parametrized job.
    
    Change-Id: I0f798d7a2659c6cd061002db151f3fa787eb6370
    Reviewed-on: http://gerrit.cloudera.org:8080/15106
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
    Reviewed-by: Tim Armstrong <ta...@cloudera.com>
---
 .../main/java/org/apache/impala/catalog/HdfsTable.java |  3 +++
 .../impala/catalog/local/DirectMetaProvider.java       |  3 +++
 .../java/org/apache/impala/util/MetaStoreUtil.java     | 18 ++++++++++++++++++
 .../java/org/apache/impala/catalog/CatalogTest.java    |  6 ++----
 .../apache/impala/catalog/PartialCatalogInfoTest.java  |  6 ++----
 .../apache/impala/catalog/local/LocalCatalogTest.java  |  6 ++----
 .../queries/QueryTest/show-create-table.test           |  2 +-
 7 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java b/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
index ae1e3d7..2d4f8d5 100644
--- a/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
@@ -1058,6 +1058,9 @@ public class HdfsTable extends Table implements FeFsTable {
           new PrimaryKeysRequest(msTbl.getDbName(), msTbl.getTableName())));
       foreignKeys_.addAll(client.getForeignKeys(new ForeignKeysRequest(null, null,
           msTbl.getDbName(), msTbl.getTableName())));
+
+      // Store primary keys in a canonical order.
+      primaryKeys_.sort(new MetaStoreUtil.SqlPrimaryKeyComparator());
     } catch (Exception e) {
       throw new TableLoadingException("Failed to load primary keys/foreign keys for "
           + "table: " + getFullName(), e);
diff --git a/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java b/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
index 64e51f4..620c966 100644
--- a/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
+++ b/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
@@ -174,6 +174,9 @@ class DirectMetaProvider implements MetaProvider {
           new PrimaryKeysRequest(msTbl.getDbName(), msTbl.getTableName())));
       foreignKeys.addAll(c.getHiveClient().getForeignKeys(new ForeignKeysRequest(null,
           null, msTbl.getDbName(), msTbl.getTableName())));
+
+      // Store primary keys in a canonical order.
+      primaryKeys.sort(new MetaStoreUtil.SqlPrimaryKeyComparator());
     }
     return new Pair<>(primaryKeys, foreignKeys);
   }
diff --git a/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java b/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
index b0c182f..54af872 100644
--- a/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
+++ b/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
@@ -18,6 +18,7 @@
 package org.apache.impala.util;
 
 import java.util.ArrayList;
+import java.util.Comparator;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -34,6 +35,7 @@ import org.apache.hadoop.hive.metastore.api.FireEventRequestData;
 import org.apache.hadoop.hive.metastore.api.InsertEventRequestData;
 import org.apache.hadoop.hive.metastore.api.MetaException;
 import org.apache.hadoop.hive.metastore.api.Partition;
+import org.apache.hadoop.hive.metastore.api.SQLPrimaryKey;
 import org.apache.hadoop.hive.metastore.api.StorageDescriptor;
 import org.apache.hadoop.hive.metastore.api.Table;
 import org.apache.impala.catalog.CatalogException;
@@ -355,4 +357,20 @@ public class MetaStoreUtil {
     Preconditions.checkNotNull(msTbl);
     return msTbl.getSd().getNumBuckets() > 0;
   }
+
+  /**
+   * A custom comparator class to sort SQLPrimaryKeys in alphabetical order of the
+   * primary key name. If the primary key names are same(composite primary key), we
+   * will sort in increasing order of key_seq.
+   */
+  public static class SqlPrimaryKeyComparator implements Comparator<SQLPrimaryKey> {
+    @Override
+    public int compare(SQLPrimaryKey pk1, SQLPrimaryKey pk2) {
+      int keyNameComp = pk1.getPk_name().compareTo(pk2.getPk_name());
+      if (keyNameComp == 0) {
+        return Integer.compare(pk1.getKey_seq(), pk2.getKey_seq());
+      }
+      return keyNameComp;
+    }
+  }
 }
diff --git a/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java b/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
index e038013..1fbc5e8 100644
--- a/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
+++ b/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
@@ -453,10 +453,8 @@ public class CatalogTest {
       assertEquals("functional", pk.getTable_db());
       assertEquals("parent_table", pk.getTable_name());
     }
-    // HMS returns the columns in the reverse order of PK columns specified in the DDL.
-    // "parent_table" in our test data has primary key(id, year) specified.
-    assertEquals("year", primaryKeys.get(0).getColumn_name());
-    assertEquals("id", primaryKeys.get(1).getColumn_name());
+    assertEquals("id", primaryKeys.get(0).getColumn_name());
+    assertEquals("year", primaryKeys.get(1).getColumn_name());
 
     // Force load parent_table_2. Required for fetching foreign keys from child_table.
     catalog_.getOrLoadTable("functional", "parent_table_2", "test");
diff --git a/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java b/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
index e5fc072..01599e7 100644
--- a/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
+++ b/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
@@ -263,10 +263,8 @@ public class PartialCatalogInfoTest {
       assertEquals("functional", pk.getTable_db());
       assertEquals("parent_table", pk.getTable_name());
     }
-    // HMS returns the columns in the reverse order of PK columns specified in the DDL.
-    // "parent_table" in our test data has primary key(id, year) specified.
-    assertEquals("year", primaryKeys.get(0).getColumn_name());
-    assertEquals("id", primaryKeys.get(1).getColumn_name());
+    assertEquals("id", primaryKeys.get(0).getColumn_name());
+    assertEquals("year", primaryKeys.get(1).getColumn_name());
 
     // Test constraints of child_table.
     req = new TGetPartialCatalogObjectRequest();
diff --git a/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java b/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
index 48be5b7..67a36d9 100644
--- a/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
+++ b/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
@@ -180,10 +180,8 @@ public class LocalCatalogTest {
       assertEquals("functional", pk.getTable_db());
       assertEquals("parent_table", pk.getTable_name());
     }
-    // HMS returns the columns in the reverse order of PK columns specified in the DDL.
-    // "parent_table" in our test data has primary key(id, year) specified.
-    assertEquals("year", primaryKeys.get(0).getColumn_name());
-    assertEquals("id", primaryKeys.get(1).getColumn_name());
+    assertEquals("id", primaryKeys.get(0).getColumn_name());
+    assertEquals("year", primaryKeys.get(1).getColumn_name());
 
     t = (FeFsTable) catalog_.getTable("functional", "child_table");
     assertNotNull(t);
diff --git a/testdata/workloads/functional-query/queries/QueryTest/show-create-table.test b/testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
index db62d67..274bff1 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
@@ -544,7 +544,7 @@ SHOW CREATE TABLE functional.parent_table
 CREATE EXTERNAL TABLE functional.parent_table (
   id INT,
   year STRING,
-  PRIMARY KEY (year, id)
+  PRIMARY KEY (id, year)
  )
 ROW FORMAT DELIMITED FIELDS TERMINATED BY ','
 WITH SERDEPROPERTIES ('field.delim'=',', 'serialization.format'=',')


[impala] 01/02: IMPALA-9242: Bump CDH_BUILD_NUMBER to 1814051

Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 641c406938c0e17c0b85fb2a5e30739a5439df1d
Author: Csaba Ringhofer <cs...@cloudera.com>
AuthorDate: Thu Jan 16 17:01:07 2020 +0100

    IMPALA-9242: Bump CDH_BUILD_NUMBER to 1814051
    
    The goal is to get SENTRY-2539 + SENTRY-1291.
    This will allow us to implement FilteredPrivilegeCache
    instead of PrivilegeCache to make Sentry access checks
    faster.
    
    Change-Id: I13a8ffcf6c8ce32f0901fc6ca35fda081fed4d06
    Reviewed-on: http://gerrit.cloudera.org:8080/15063
    Reviewed-by: Quanlong Huang <hu...@gmail.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 bin/impala-config.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bin/impala-config.sh b/bin/impala-config.sh
index 7601ada..1ad556f 100755
--- a/bin/impala-config.sh
+++ b/bin/impala-config.sh
@@ -166,7 +166,7 @@ fi
 
 : ${IMPALA_TOOLCHAIN_HOST:=native-toolchain.s3.amazonaws.com}
 export IMPALA_TOOLCHAIN_HOST
-export CDH_BUILD_NUMBER=1582079
+export CDH_BUILD_NUMBER=1814051
 export CDH_MAVEN_REPOSITORY=\
 "https://${IMPALA_TOOLCHAIN_HOST}/build/cdh_components/${CDH_BUILD_NUMBER}/maven"
 export CDH_HADOOP_VERSION=3.0.0-cdh6.x-SNAPSHOT