IMPALA-295: impala not populating hive metadata correctly for create table

be/src/service/impala-beeswax-server.cc

@@ -152,6 +152,7 @@ void ImpalaServer::query(QueryHandle& query_handle, const Query& query) {
     // something is run.
     lock_guard<mutex> l(session->lock);
     if (session->user.empty()) session->user = query.hadoop_user;
+    query_request.sessionState.user = session->user;
   }
   status = Execute(query_request, session, query_request.sessionState, &exec_state);
 

be/src/service/impala-server.cc

@@ -1874,6 +1874,7 @@ void ImpalaServer::TQueryOptionsToMap(const TQueryOptions& query_option,
 void ImpalaServer::SessionState::ToThrift(TSessionState* state) {
   lock_guard<mutex> l(lock);
   state->database = database;
+  state->user = user;
 }
 
 void ImpalaServer::MembershipCallback(

common/thrift/Frontend.thrift

@@ -263,14 +263,17 @@ struct TCreateTableLikeParams {
   // Do not throw an error if a table of the same name already exists.
   4: required bool if_not_exists
 
+  // Owner of the table
+  5: required string owner
+
   // Optional file format for this table
-  5: optional TFileFormat file_format
+  6: optional TFileFormat file_format
 
   // Optional comment for the table
-  6: optional string comment
+  7: optional string comment
 
   // Optional storage location for the table
-  7: optional string location
+  8: optional string location
 }
 
 // Parameters of CREATE TABLE commands
@@ -287,22 +290,25 @@ struct TCreateTableParams {
   // The file format for this table
   4: required TFileFormat file_format
 
-  // Specifies how rows and columns are interpreted when reading data from the table
-  5: optional TTableRowFormat row_format
-
   // True if the table is an "EXTERNAL" table. Dropping an external table will NOT remove
   // table data from the file system. If EXTERNAL is not specified, all table data will be
   // removed when the table is dropped.
-  6: required bool is_external
+  5: required bool is_external
 
   // Do not throw an error if a table of the same name already exists.
-  7: required bool if_not_exists
+  6: required bool if_not_exists
+
+  // The owner of the table
+  7: required string owner
+
+  // Specifies how rows and columns are interpreted when reading data from the table
+  8: optional TTableRowFormat row_format
 
   // Optional comment for the table
-  8: optional string comment
+  9: optional string comment
 
   // Optional storage location for the table
-  9: optional string location
+  10: optional string location
 }
 
 // Parameters of DROP DATABASE commands
@@ -327,6 +333,9 @@ struct TDropTableParams {
 struct TSessionState {
   // The default database, changed by USE <database> queries.
   1: required string database
+
+  // The user who this session belongs to.
+  2: required string user
 }
 
 struct TClientRequest {

fe/src/main/java/com/cloudera/impala/analysis/AnalysisContext.java

@@ -33,16 +33,16 @@ public class AnalysisContext {
   // The name of the database to use if one is not explicitly specified by a query.
   private final String defaultDatabase;
 
+  // The user who initiated the request.
+  private final String user;
+
   private final TQueryGlobals queryGlobals;
 
-  public AnalysisContext(Catalog catalog, String defaultDb) {
+  public AnalysisContext(Catalog catalog, String defaultDb, String user) {
     this.catalog = catalog;
-    defaultDatabase = defaultDb;
-    queryGlobals = createQueryGlobals();
-  }
-
-  public AnalysisContext(Catalog catalog) {
-    this(catalog, Catalog.DEFAULT_DB);
+    this.defaultDatabase = defaultDb;
+    this.user = user;
+    this.queryGlobals = createQueryGlobals();
   }
 
   static public class AnalysisResult {
@@ -194,7 +194,7 @@ public class AnalysisContext {
       if (result.stmt == null) {
         return null;
       }
-      result.analyzer = new Analyzer(catalog, defaultDatabase, queryGlobals);
+      result.analyzer = new Analyzer(catalog, defaultDatabase, user, queryGlobals);
       result.stmt.analyze(result.analyzer);
       return result;
     } catch (AnalysisException e) {

fe/src/main/java/com/cloudera/impala/analysis/Analyzer.java

@@ -52,6 +52,7 @@ public class Analyzer {
   private final DescriptorTable descTbl;
   private final Catalog catalog;
   private final String defaultDb;
+  private final String user;
   private final IdGenerator<ExprId> conjunctIdGenerator;
   private final TQueryGlobals queryGlobals;
 
@@ -110,14 +111,17 @@ public class Analyzer {
    * @param catalog
    */
   public Analyzer(Catalog catalog) {
-    this(catalog, Catalog.DEFAULT_DB, new TQueryGlobals());
+    this(catalog, Catalog.DEFAULT_DB, System.getProperty("user.name"),
+        new TQueryGlobals());
   }
 
-  public Analyzer(Catalog catalog, String defaultDb, TQueryGlobals queryGlobals) {
+  public Analyzer(Catalog catalog, String defaultDb, String user,
+        TQueryGlobals queryGlobals) {
     this.parentAnalyzer = null;
     this.catalog = catalog;
     this.descTbl = new DescriptorTable();
     this.defaultDb = defaultDb;
+    this.user = user;
     this.conjunctIdGenerator = new IdGenerator<ExprId>();
     this.queryGlobals = queryGlobals;
   }
@@ -132,6 +136,7 @@ public class Analyzer {
     this.catalog = parentAnalyzer.catalog;
     this.descTbl = parentAnalyzer.descTbl;
     this.defaultDb = parentAnalyzer.defaultDb;
+    this.user = parentAnalyzer.user;
     // make sure we don't create duplicate ids across entire stmt
     this.conjunctIdGenerator = parentAnalyzer.conjunctIdGenerator;
     this.queryGlobals = parentAnalyzer.queryGlobals;
@@ -636,6 +641,10 @@ public class Analyzer {
     return defaultDb;
   }
 
+  public String getUser() {
+    return user;
+  }
+
   public TQueryGlobals getQueryGlobals() {
     return queryGlobals;
   }

fe/src/main/java/com/cloudera/impala/analysis/CreateTableLikeStmt.java

@@ -45,6 +45,7 @@ public class CreateTableLikeStmt extends ParseNodeBase {
   // Set during analysis
   private String dbName;
   private String srcDbName;
+  private String owner;
 
   /**
    * Builds a CREATE TABLE LIKE statement
@@ -116,6 +117,11 @@ public class CreateTableLikeStmt extends ParseNodeBase {
     return location;
   }
 
+  public String getOwner() {
+    Preconditions.checkNotNull(owner);
+    return owner;
+  }
+
   public String debugString() {
     return toSql();
   }
@@ -153,6 +159,7 @@ public class CreateTableLikeStmt extends ParseNodeBase {
     TCreateTableLikeParams params = new TCreateTableLikeParams();
     params.setTable_name(new TTableName(getDb(), getTbl()));
     params.setSrc_table_name(new TTableName(getSrcDb(), getSrcTbl()));
+    params.setOwner(getOwner());
     params.setIs_external(isExternal());
     params.setComment(comment);
     if (fileFormat != null) {
@@ -168,6 +175,7 @@ public class CreateTableLikeStmt extends ParseNodeBase {
     dbName = tableName.isFullyQualified() ? tableName.getDb() : analyzer.getDefaultDb();
     srcDbName =
         srcTableName.isFullyQualified() ? srcTableName.getDb() : analyzer.getDefaultDb();
+    owner = analyzer.getUser();
 
     if (analyzer.getCatalog().getDb(dbName) == null) {
       throw new AnalysisException("Database does not exist: " + dbName);

fe/src/main/java/com/cloudera/impala/analysis/CreateTableStmt.java

@@ -44,6 +44,7 @@ public class CreateTableStmt extends ParseNodeBase {
 
   // Set during analysis
   private String dbName;
+  private String owner;
 
   /**
    * Builds a CREATE TABLE statement
@@ -119,6 +120,11 @@ public class CreateTableStmt extends ParseNodeBase {
     return fileFormat;
   }
 
+  public String getOwner() {
+    Preconditions.checkNotNull(owner);
+    return owner;
+  }
+
   public RowFormat getRowFormat() {
     return rowFormat;
   }
@@ -183,6 +189,7 @@ public class CreateTableStmt extends ParseNodeBase {
     for (ColumnDef col: getPartitionColumnDefs()) {
       params.addToPartition_columns(col.toThrift());
     }
+    params.setOwner(getOwner());
     params.setIs_external(isExternal());
     params.setComment(comment);
     params.setLocation(location);
@@ -196,6 +203,7 @@ public class CreateTableStmt extends ParseNodeBase {
   public void analyze(Analyzer analyzer) throws AnalysisException {
     Preconditions.checkState(tableName != null && !tableName.isEmpty());
     dbName = tableName.isFullyQualified() ? tableName.getDb() : analyzer.getDefaultDb();
+    owner = analyzer.getUser();
 
     if (analyzer.getCatalog().getDb(dbName) == null) {
       throw new AnalysisException("Database does not exist: " + dbName);

fe/src/main/java/com/cloudera/impala/catalog/Catalog.java

@@ -725,6 +725,7 @@ public class Catalog {
    * @param tableName - Fully qualified name of the new table.
    * @param column - List of column definitions for the new table.
    * @param partitionColumn - List of partition column definitions for the new table.
+   * @param owner - Owner of this table.
    * @param isExternal
    *    If true, table is created as external which means the data will not be deleted
    *    if dropped. External tables can also be created on top of existing data.
@@ -734,7 +735,7 @@ public class Catalog {
    * @param ifNotExists - If true, no errors are thrown if the table already exists
    */
   public void createTable(TableName tableName, List<TColumnDef> columns,
-      List<TColumnDef> partitionColumns, boolean isExternal, String comment,
+      List<TColumnDef> partitionColumns, String owner, boolean isExternal, String comment,
       RowFormat rowFormat, FileFormat fileFormat, String location, boolean ifNotExists)
       throws MetaException, NoSuchObjectException, AlreadyExistsException,
       InvalidObjectException, org.apache.thrift.TException {
@@ -750,6 +751,7 @@ public class Catalog {
         new org.apache.hadoop.hive.metastore.api.Table();
     tbl.setDbName(tableName.getDb());
     tbl.setTableName(tableName.getTbl());
+    tbl.setOwner(owner);
     tbl.setParameters(new HashMap<String, String>());
 
     if (comment != null) {
@@ -758,6 +760,8 @@ public class Catalog {
     if (isExternal) {
       tbl.setTableType(TableType.EXTERNAL_TABLE.toString());
       tbl.putToParameters("EXTERNAL", "TRUE");
+    } else {
+      tbl.setTableType(TableType.MANAGED_TABLE.toString());
     }
 
     StorageDescriptor sd = HiveStorageDescriptorFactory.createSd(fileFormat, rowFormat);
@@ -785,6 +789,7 @@ public class Catalog {
    *
    * @param tableName - Fully qualified name of the new table.
    * @param srcTableName - Fully qualified name of the old table.
+   * @param owner - Owner of this table.
    * @param isExternal
    *    If true, table is created as external which means the data will not be deleted
    *    if dropped. External tables can also be created on top of existing data.
@@ -796,7 +801,7 @@ public class Catalog {
    *                   default location.
    * @param ifNotExists - If true, no errors are thrown if the table already exists
    */
-  public void createTableLike(TableName tableName, TableName srcTableName,
+  public void createTableLike(TableName tableName, TableName srcTableName, String owner,
       boolean isExternal, String comment, FileFormat fileFormat, String location,
       boolean ifNotExists) throws MetaException, NoSuchObjectException,
       AlreadyExistsException, InvalidObjectException, org.apache.thrift.TException,
@@ -813,6 +818,7 @@ public class Catalog {
         srcTable.getMetaStoreTable().deepCopy();
     tbl.setDbName(tableName.getDb());
     tbl.setTableName(tableName.getTbl());
+    tbl.setOwner(owner);
     if (tbl.getParameters() == null) {
       tbl.setParameters(new HashMap<String, String>());
     }

fe/src/main/java/com/cloudera/impala/service/Frontend.java

@@ -277,24 +277,24 @@ public class Frontend {
    * Creates a new table in the metastore.
    */
   public void createTable(TableName tableName, List<TColumnDef> columns,
-      List<TColumnDef> partitionColumns, boolean isExternal, String comment,
+      List<TColumnDef> partitionColumns, String owner, boolean isExternal, String comment,
       RowFormat rowFormat, FileFormat fileFormat, String location, boolean ifNotExists)
       throws MetaException, NoSuchObjectException, org.apache.thrift.TException,
       AlreadyExistsException, InvalidObjectException {
-    catalog.createTable(tableName, columns, partitionColumns, isExternal, comment,
+    catalog.createTable(tableName, columns, partitionColumns, owner, isExternal, comment,
         rowFormat, fileFormat, location, ifNotExists);
   }
 
   /**
    * Creates a new table in the metastore.
    */
-  public void createTableLike(TableName tableName, TableName oldTableName,
+  public void createTableLike(TableName tableName, TableName oldTableName, String owner,
       boolean isExternal, String comment, FileFormat fileFormat, String location,
       boolean ifNotExists) throws MetaException, NoSuchObjectException,
       org.apache.thrift.TException, AlreadyExistsException, InvalidObjectException,
       ImpalaException, TableLoadingException {
-    catalog.createTableLike(tableName, oldTableName, isExternal, comment, fileFormat,
-        location, ifNotExists);
+    catalog.createTableLike(tableName, oldTableName, owner, isExternal, comment,
+        fileFormat, location, ifNotExists);
   }
 
   /**
@@ -387,8 +387,8 @@ public class Frontend {
   public TExecRequest createExecRequest(
       TClientRequest request, StringBuilder explainString)
       throws InternalException, AnalysisException, NotImplementedException {
-    AnalysisContext analysisCtxt =
-        new AnalysisContext(catalog, request.sessionState.database);
+    AnalysisContext analysisCtxt = new AnalysisContext(catalog,
+        request.sessionState.database, request.sessionState.user);
     AnalysisContext.AnalysisResult analysisResult = null;
     LOG.info("analyze query " + request.stmt);
     try {

fe/src/main/java/com/cloudera/impala/service/JniFrontend.java

@@ -215,8 +215,9 @@ public class JniFrontend {
     TCreateTableParams params = new TCreateTableParams();
     deserializeThrift(params, thriftCreateTableParams);
     frontend.createTable(TableName.fromThrift(params.getTable_name()),
-        params.getColumns(), params.getPartition_columns(), params.isIs_external(),
-        params.getComment(), RowFormat.fromThrift(params.getRow_format()),
+        params.getColumns(), params.getPartition_columns(), params.getOwner(),
+        params.isIs_external(), params.getComment(),
+        RowFormat.fromThrift(params.getRow_format()),
         FileFormat.fromThrift(params.getFile_format()),
         params.getLocation(), params.isIf_not_exists());
   }
@@ -236,7 +237,7 @@ public class JniFrontend {
       comment = params.getComment();
     }
     frontend.createTableLike(TableName.fromThrift(params.getTable_name()),
-        TableName.fromThrift(params.getSrc_table_name()),
+        TableName.fromThrift(params.getSrc_table_name()), params.getOwner(),
         params.isIs_external(), comment, fileFormat, params.getLocation(),
         params.isIf_not_exists());
   }

fe/src/test/java/com/cloudera/impala/analysis/ToSqlTest.java

@@ -25,7 +25,8 @@ public class ToSqlTest {
 
   private static AnalysisContext.AnalysisResult analyze(String query) {
     try {
-      AnalysisContext analysisCtxt = new AnalysisContext(catalog);
+      AnalysisContext analysisCtxt = new AnalysisContext(catalog, catalog.DEFAULT_DB,
+          System.getProperty("user.name"));
       AnalysisContext.AnalysisResult analysisResult = analysisCtxt.analyze(query);
       Preconditions.checkNotNull(analysisResult.getStmt());
       return analysisResult;

fe/src/test/java/com/cloudera/impala/catalog/CatalogTest.java

@@ -16,6 +16,7 @@ import java.util.Set;
 
 import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.TableType;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -383,6 +384,21 @@ public class CatalogTest {
     assertNull(nonExistentDb);
   }
 
+  @Test
+  public void testCreateTableMetadata() throws TableLoadingException {
+    Table table = catalog.getDb("functional").getTable("alltypes");
+    // Tables are created via Impala so the metadata should have been populated properly.
+    // alltypes is an external table.
+    assertEquals(System.getProperty("user.name"), table.getMetaStoreTable().getOwner());
+    assertEquals(TableType.EXTERNAL_TABLE.toString(),
+        table.getMetaStoreTable().getTableType());
+    // alltypesinsert is created using CREATE TABLE LIKE and is a MANAGED table
+    table = catalog.getDb("functional").getTable("alltypesinsert");
+    assertEquals(System.getProperty("user.name"), table.getMetaStoreTable().getOwner());
+    assertEquals(TableType.MANAGED_TABLE.toString(),
+        table.getMetaStoreTable().getTableType());
+  }
+
   @Test
   public void testLoadingUnsupportedTableTypes() {
     try {

fe/src/test/java/com/cloudera/impala/planner/PlannerTest.java

@@ -166,7 +166,8 @@ public class PlannerTest {
     boolean isImplemented = expectedErrorMsg == null;
 
     options.setNum_nodes(1);
-    TSessionState sessionState = new TSessionState("default");
+    TSessionState sessionState =
+        new TSessionState("default", System.getProperty("user.name"));
     TClientRequest request = new TClientRequest(query, options, sessionState);
     StringBuilder explainBuilder = new StringBuilder();