You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by robertamarton <gi...@git.apache.org> on 2018/04/10 18:41:12 UTC
[GitHub] trafodion pull request #1512: Get statement enhancements
GitHub user robertamarton opened a pull request:
https://github.com/apache/trafodion/pull/1512
Get statement enhancements
Added support and privilege checks for the following commands:
get functions for library
get procedures for library
get table_mapping functions for library
get indexes on table
get objects on table
get views on table
get views on view
get libraries in schema
get objects in view
get tables in view
get views in view
get indexes for user
get tables for user
get libraries for user
get views for user
Changed "get libraries for schema" to include libraries where the current user
has execute privilege on one of the libraries routines (functions, procedures,
or table_mapping functions).
Addressed a performance issue when determining if the user has column level
privileges. If the user has granted privileges against native Hive tables
through EsgynDB, we need to get the column name from Hive. The call to get the
column, by calling hivemd, is very expensive. This change checks to see if the
requested user has been granted any column level privileges on a hive table.
If so, we will go ahead and do the mapping (call hivemd). If not, then we will
not include the hivemd fragment for the query. Since we are scanning the column
privileges table anyway, we also see if the requested user (or their roles) has
been granted any privileges. If so, we include the column privileges check in
the query.
Commented out get statements that we do not support at this time.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/robertamarton/incubator-trafodion get-stmts
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafodion/pull/1512.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1512
----
commit 021faec6a18336cfefd11b30d100a8fe4e3da1ee
Author: Roberta Marton <rm...@...>
Date: 2018-04-10T18:35:39Z
Get statement enhancements
Added support and privilege checks for the following commands:
get functions for library
get procedures for library
get table_mapping functions for library
get indexes on table
get objects on table
get views on table
get views on view
get libraries in schema
get objects in view
get tables in view
get views in view
get indexes for user
get tables for user
get libraries for user
get views for user
Changed "get libraries for schema" to include libraries where the current user
has execute privilege on one of the libraries routines (functions, procedures,
or table_mapping functions).
Addressed a performance issue when determining if the user has column level
privileges. If the user has granted privileges against native Hive tables
through EsgynDB, we need to get the column name from Hive. The call to get the
column, by calling hivemd, is very expensive. This change checks to see if the
requested user has been granted any column level privileges on a hive table.
If so, we will go ahead and do the mapping (call hivemd). If not, then we will
not include the hivemd fragment for the query. Since we are scanning the column
privileges table anyway, we also see if the requested user (or their roles) has
been granted any privileges. If so, we include the column privileges check in
the query.
Commented out get statements that we do not support at this time.
----
---
[GitHub] trafodion pull request #1512: [TRAFODION-2175] Get statement enhancements
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/trafodion/pull/1512
---
[GitHub] trafodion pull request #1512: [TRAFODION-2175] Get statement enhancements
Posted by robertamarton <gi...@git.apache.org>.
Github user robertamarton commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1512#discussion_r180791492
--- Diff: core/sql/executor/ExExeUtilGet.cpp ---
@@ -1642,6 +1672,119 @@ NABoolean ExExeUtilGetMetadataInfoTcb::checkUserPrivs(
return TRUE;
}
+// ----------------------------------------------------------------------------
+// method: colPrivsFrag
+//
+// This method was added to address a performance issue. When determining if
+// the user has column level privileges, we need to get the column name from
+// Hive. The call to get the column name (hivemd) is very expensive. So this
+// method checks to see if the requested user has been granted any column
+// level privileges on a hive table. If so, we will go ahead and do the
+// mapping (call hivemd). If not, then we will not include the hivemd
+// fragment for the query.
+//
+// Since we are scanning the column privileges table anyway, we also see if
+// the requested user (or their roles) has been granted any privileges. If so,
+// we include the column privileges check in the query.
+//
+// For Sentry enabled installations, we won't store Hive privileges in
+// EsgynDB metadata. By avoiding the hivemd calls, we save a lot of time
+// in processing the request.
+//
+// returns additional union(s) for the getPrivForAuth query
+// returns:
+// 0 - successful
+// -1 - unexpected error occurred
+// ----------------------------------------------------------------------------
+Int32 ExExeUtilGetMetadataInfoTcb::colPrivsFrag(
+ const char *authName,
+ const char * cat,
+ const NAString &privWhereClause,
+ NAString &colPrivsStmt)
+{
+ // if no authorization, skip
+ if (!CmpCommon::context()->isAuthorizationEnabled())
+ return 0;
+
+ short rc = 0;
+ Lng32 cliRC = 0;
+
+ // See if privileges granted on Hive object or to the user/user's roles
+ NAString likeClause("like 'HIVE.%'");
+ sprintf(queryBuf_, "select "
+ "sum(case when (object_name %s and grantee_id %s) then 1 else 0 end), "
+ "sum(case when grantee_id %s then 1 else 0 end) "
+ "from %s.\"%s\".%s",
+ likeClause.data(), privWhereClause.data(), privWhereClause.data(),
+ cat, SEABASE_PRIVMGR_SCHEMA,
+ PRIVMGR_COLUMN_PRIVILEGES);
+
+ if (initializeInfoList(infoList_)) return -1;
+
+ numOutputEntries_ = 2;
+ cliRC = fetchAllRows(infoList_, queryBuf_, numOutputEntries_, FALSE, rc);
+ if (cliRC < 0)
+ {
+ cliInterface()->retrieveSQLDiagnostics(getDiagsArea());
+ return -1;
+ }
+
+ bool hasHive = false;
+ bool hasGrants = false;
+ infoList_->position();
+ OutputInfo * vi = (OutputInfo*)infoList_->getCurr();
+ if (vi && vi->get(0))
+ {
+ if (*(Lng32*)vi->get(0) > 0)
--- End diff --
I will change these to int64 for a future delivery.
---
[GitHub] trafodion pull request #1512: Get statement enhancements
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1512#discussion_r180558802
--- Diff: core/sql/executor/ExExeUtilGet.cpp ---
@@ -1642,6 +1672,119 @@ NABoolean ExExeUtilGetMetadataInfoTcb::checkUserPrivs(
return TRUE;
}
+// ----------------------------------------------------------------------------
+// method: colPrivsFrag
+//
+// This method was added to address a performance issue. When determining if
+// the user has column level privileges, we need to get the column name from
+// Hive. The call to get the column name (hivemd) is very expensive. So this
+// method checks to see if the requested user has been granted any column
+// level privileges on a hive table. If so, we will go ahead and do the
+// mapping (call hivemd). If not, then we will not include the hivemd
+// fragment for the query.
+//
+// Since we are scanning the column privileges table anyway, we also see if
+// the requested user (or their roles) has been granted any privileges. If so,
+// we include the column privileges check in the query.
+//
+// For Sentry enabled installations, we won't store Hive privileges in
+// EsgynDB metadata. By avoiding the hivemd calls, we save a lot of time
+// in processing the request.
+//
+// returns additional union(s) for the getPrivForAuth query
+// returns:
+// 0 - successful
+// -1 - unexpected error occurred
+// ----------------------------------------------------------------------------
+Int32 ExExeUtilGetMetadataInfoTcb::colPrivsFrag(
+ const char *authName,
+ const char * cat,
+ const NAString &privWhereClause,
+ NAString &colPrivsStmt)
+{
+ // if no authorization, skip
+ if (!CmpCommon::context()->isAuthorizationEnabled())
+ return 0;
+
+ short rc = 0;
+ Lng32 cliRC = 0;
+
+ // See if privileges granted on Hive object or to the user/user's roles
+ NAString likeClause("like 'HIVE.%'");
+ sprintf(queryBuf_, "select "
+ "sum(case when (object_name %s and grantee_id %s) then 1 else 0 end), "
+ "sum(case when grantee_id %s then 1 else 0 end) "
+ "from %s.\"%s\".%s",
+ likeClause.data(), privWhereClause.data(), privWhereClause.data(),
+ cat, SEABASE_PRIVMGR_SCHEMA,
+ PRIVMGR_COLUMN_PRIVILEGES);
+
+ if (initializeInfoList(infoList_)) return -1;
+
+ numOutputEntries_ = 2;
+ cliRC = fetchAllRows(infoList_, queryBuf_, numOutputEntries_, FALSE, rc);
+ if (cliRC < 0)
+ {
+ cliInterface()->retrieveSQLDiagnostics(getDiagsArea());
+ return -1;
+ }
+
+ bool hasHive = false;
+ bool hasGrants = false;
+ infoList_->position();
+ OutputInfo * vi = (OutputInfo*)infoList_->getCurr();
+ if (vi && vi->get(0))
+ {
+ if (*(Lng32*)vi->get(0) > 0)
--- End diff --
I experimented with a SUM aggregate and found that the output is a 64-bit integer. So, *(Lng32*) might give a wrong answer if we ever have someone with 2^32 privileges (admittedly unlikely).
---