You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hawq.apache.org by kavinderd <gi...@git.apache.org> on 2016/03/24 00:11:04 UTC
[GitHub] incubator-hawq pull request: Writable security fix
GitHub user kavinderd opened a pull request:
https://github.com/apache/incubator-hawq/pull/497
Writable security fix
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kavinderd/incubator-hawq writable_security_fix
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/497.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 #497
----
commit 50926e7470e4d5a6c6857d57063f8b8f5634d6f8
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-03-04T18:49:13Z
HAWQ-462. Adding dfs_address from pgfilespace along with fragment data sent to segments
commit c5d3e4a3ba232f69b217f112c9908369d2f2ce2e
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-03-04T21:41:42Z
HAWQ-462. CR fixes
commit f996eb8a69a565ac8f7afd202247bf27ae92c0ff
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-03-08T19:48:58Z
HAWQ-462. Moved dfs address from fragment to segment metadata
commit 07d31d33f8f7cebf735a1d9599a104c0f55daf03
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-03-08T20:03:55Z
HAWQ-462. Fix compilation error
commit 287a7f0aa25fa806c259b7c7a68cc8c89c78045d
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-03-08T22:02:22Z
HAWQ-462. Configure dfs_address only if secure
commit 49dfdd87a91bf046c8c859bfaf12c7d26c19445c
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-03-09T18:49:24Z
HAWQ-462. CR fixes
commit c0d484e16bd27777780de5b71e38e129faea34f0
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-03-17T23:54:37Z
Merge branch 'HAWQ-462' of https://github.com/apache/incubator-hawq into HAWQ-462
commit 55147857cf6cd7ea6552caea24920b58f95a841d
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-03-23T03:43:08Z
Work in progress, pass namespace to segment and eventually to pxf
commit dd3da9152b43d105050da5743ce57fcb8920598c
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-03-23T21:40:49Z
fixes for memory allocation issue
commit 8a95f8e5ea139bda9f58b3b23fc9622c3c35d1a6
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-03-23T21:54:23Z
cleanup
commit e88cb5334db62fc993dbbbed454138e7b98165c6
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-03-23T21:57:20Z
Merge with HAWQ-462
commit 5cb423d500201abfeefe05812116737477f94149
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-03-23T22:48:35Z
fix to passing char by ref
commit 4a4c460d626560cf0caa580a18acd5827c666b15
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-03-23T23:07:32Z
Remove unecessary fields
commit 3d33f49f12b2dfc25a8802af42dc78ccfa78af15
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-03-23T23:09:56Z
Use const for delim
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57267290
--- Diff: src/include/access/fileam.h ---
@@ -44,12 +44,13 @@
* ExternalInsertDescData is used for storing state related
* to inserting data into a writable external table.
*/
-typedef struct ExternalInsertDescData
+typedef struct ExternalInsertDescDat
--- End diff --
typo?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57267606
--- Diff: src/bin/gpfusion/gpbridgeapi.c ---
@@ -273,6 +277,26 @@ void gpbridge_export_start(PG_FUNCTION_ARGS)
}
+void parse_namespace(char *uri, char** extracted_namespace){
+ int i = 0;
+ int len = strlen(uri);
+
+ while(uri[i] != '*' && i < len){
--- End diff --
Actually, shouldn't we parse the URI as part of the parse function we already have? that way we can know that we parsed everything else before we expect to read the delimiter and the dfs_address - similar to the way it's implemented right now, when we parse dfs_address that was appended by hd_work_mgr.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd closed the pull request at:
https://github.com/apache/incubator-hawq/pull/497
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on the pull request:
https://github.com/apache/incubator-hawq/pull/497#issuecomment-200624680
Not sure I understood everything, so correct me if I am wrong.
So right now we have two flows
1. For readable tables we append dfs_address in hd_work_mgr and parse it in pxfuriparser.
2. For writable tables we append dfs_address as part of the URI that is part of the external table context. Then we parse it in gpbridgeapi, but only in the export case (writable).
My question is - shouldn't we consolidate the two solutions and adopt the 2nd flow for both readable and writable?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57322038
--- Diff: src/bin/gpfusion/gpbridgeapi.c ---
@@ -273,6 +277,26 @@ void gpbridge_export_start(PG_FUNCTION_ARGS)
}
+void parse_namespace(char *uri, char** extracted_namespace){
+ int i = 0;
+ int len = strlen(uri);
+
+ while(uri[i] != '*' && i < len){
+ i++;
+ }
+
+ uri[i++] = '\0';
+ if(i < len){
+ *extracted_namespace = palloc(len - i + 1);
+ int extracted_namespace_len = len - i;
+ int j = 0;
+ while(i < len && j < extracted_namespace_len){
--- End diff --
Ok, good to know
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57267323
--- Diff: src/bin/gpupgrade/crc32c.c ---
@@ -0,0 +1 @@
+../../../src/backend/utils/hash/crc32c.c
--- End diff --
why are these 3 files needed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57322012
--- Diff: src/bin/gpfusion/gpbridgeapi.c ---
@@ -273,6 +277,26 @@ void gpbridge_export_start(PG_FUNCTION_ARGS)
}
+void parse_namespace(char *uri, char** extracted_namespace){
+ int i = 0;
+ int len = strlen(uri);
+
+ while(uri[i] != '*' && i < len){
--- End diff --
Good point on the delimiter, I'll replace it.
I chose not to parse the URI in `parseGPHDUri` because even with this fix we will support two different flows for the readable and writable case. So this is a temporary solution whose effects I wanted to contain only to writable cases.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57266715
--- Diff: src/backend/cdb/cdbquerycontextdispatching.c ---
@@ -313,6 +313,22 @@ FinalizeQueryContextInfo(QueryContextInfo *cxt)
WriteData(cxt, buffer.data, buffer.len);
WriteData(cxt, credential, size);
}
+
+ }
+
+ //Add Namespace for PXF
+ if(Gp_role != GP_ROLE_EXECUTE){
+ const char *namespace = cxt->sharedPath;
+ int size = strlen(namespace);
--- End diff --
indentation look off...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57322793
--- Diff: src/backend/access/external/hd_work_mgr.c ---
@@ -768,6 +769,16 @@ make_allocation_output_string(List *segment_fragments)
initStringInfo(&segwork);
appendStringInfoString(&segwork, SEGWORK_PREFIX);
+ if (enable_secure_filesystem)
--- End diff --
Currently the implementation to add the dfs_address through the external framework is not triggered in the readable case. Ideally, yes, the same implementation would apply to both. So we would like to get this fix in, tested, and then work on consolidating them.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by GodenYao <gi...@git.apache.org>.
Github user GodenYao commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57397489
--- Diff: src/bin/gpfusion/gpbridgeapi.c ---
@@ -508,17 +531,14 @@ size_t fill_buffer(gphadoop_context* context, char* start, size_t size)
void add_delegation_token(PxfInputData *inputData)
--- End diff --
can we just pass inputData->gphduri->dfs_address ? instead of the entire object?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd commented on the pull request:
https://github.com/apache/incubator-hawq/pull/497#issuecomment-201477135
Closing since https://github.com/apache/incubator-hawq/pull/503 is a more complete solution
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57267124
--- Diff: src/bin/gpfusion/gpbridgeapi.c ---
@@ -273,6 +277,26 @@ void gpbridge_export_start(PG_FUNCTION_ARGS)
}
+void parse_namespace(char *uri, char** extracted_namespace){
+ int i = 0;
+ int len = strlen(uri);
+
+ while(uri[i] != '*' && i < len){
--- End diff --
`*` can be part of the PXF location (URL), because we support regex matching for file names (e.g. `file*.txt`).
If this is going to be a permanent part of the URL (not just for secured env), perhaps we can scan from the end?
Another option is to replace the delimiter.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57266695
--- Diff: src/backend/cdb/cdbquerycontextdispatching.c ---
@@ -313,6 +313,22 @@ FinalizeQueryContextInfo(QueryContextInfo *cxt)
WriteData(cxt, buffer.data, buffer.len);
WriteData(cxt, credential, size);
}
+
+ }
+
+ //Add Namespace for PXF
--- End diff --
please change comment to c standard `/* */`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57267229
--- Diff: src/bin/gpfusion/gpbridgeapi.c ---
@@ -273,6 +277,26 @@ void gpbridge_export_start(PG_FUNCTION_ARGS)
}
+void parse_namespace(char *uri, char** extracted_namespace){
+ int i = 0;
+ int len = strlen(uri);
+
+ while(uri[i] != '*' && i < len){
+ i++;
+ }
+
+ uri[i++] = '\0';
+ if(i < len){
+ *extracted_namespace = palloc(len - i + 1);
+ int extracted_namespace_len = len - i;
+ int j = 0;
+ while(i < len && j < extracted_namespace_len){
--- End diff --
I think this loop can be replaced by `pnstrdup`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57267295
--- Diff: src/include/access/fileam.h ---
@@ -44,12 +44,13 @@
* ExternalInsertDescData is used for storing state related
* to inserting data into a writable external table.
*/
-typedef struct ExternalInsertDescData
+typedef struct ExternalInsertDescDat
{
Relation ext_rel;
FILE* ext_file;
char* ext_uri; /* "command:<cmd>" or "tablespace:<path>" */
bool ext_noop; /* no op. this segdb needs to do nothing (e.g. mirror seg) */
+ char* ext_namespace; /* Namespace data */
--- End diff --
indentation looks off
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57324733
--- Diff: src/backend/cdb/cdbquerycontextdispatching.c ---
@@ -102,7 +102,7 @@ int QueryContextDispatchingSizeMemoryLimit = 100 * 1024; /* KB */
enum QueryContextDispatchingItemType
{
- MasterXid, TablespaceLocation, TupleType, EmptyTable, FileSystemCredential
+ MasterXid, TablespaceLocation, TupleType, EmptyTable, FileSystemCredential, Namespace
--- End diff --
So these enums identify portions of the `buffer` in `QueryContextInfo`. When the segment receives the data it checks for the presence of the enums and then rebuilds the data accordingly.
The issue with dispatching the namespace from master to segment to pxf was two fold:
1. Master needs to serialize the data, segment needs to deserialize it
2. Segement needs to hold the `dfs_address` in a variable until the moment it actually invokes the pxf udf
The reason I added the namespace to the URL was so that I wouldn't need to add a new field to the `PG_FUNCTION_ARGS`.
I agree that adding namespace to URL is a bit of a hack, and this whole implementation is really the shortest path to get the namespace from master to segment so it definitely needs improvement.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd closed the pull request at:
https://github.com/apache/incubator-hawq/pull/497
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57267242
--- Diff: src/bin/gpfusion/gpbridgeapi.c ---
@@ -508,17 +532,16 @@ size_t fill_buffer(gphadoop_context* context, char* start, size_t size)
void add_delegation_token(PxfInputData *inputData)
{
PxfHdfsTokenData *token = NULL;
- char* dfs_address = NULL;
+ char *dfs_address = inputData->gphduri->dfs_address;
if (!enable_secure_filesystem)
return;
token = palloc0(sizeof(PxfHdfsTokenData));
- get_hdfs_location_from_filespace(&dfs_address);
-
- elog(DEBUG2, "locating token for %s", dfs_address);
+// get_hdfs_location_from_filespace(&dfs_address);
--- End diff --
remove :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57326498
--- Diff: src/backend/gpopt/ivy.xml ---
@@ -38,7 +38,7 @@ under the License.
</configurations>
<dependencies>
- <dependency org="emc" name="optimizer" rev="1.623" conf="osx106_x86->osx106_x86_32;osx106_x86_32->osx106_x86_32;rhel5_x86_64->rhel5_x86_64;suse10_x86_64->suse10_x86_64" />
+ <dependency org="emc" name="optimizer" rev="1.625" conf="osx106_x86->osx106_x86_32;osx106_x86_32->osx106_x86_32;rhel5_x86_64->rhel5_x86_64;suse10_x86_64->suse10_x86_64" />
--- End diff --
No, local change needed to get HAWQ to build. Will remove
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57267450
--- Diff: src/backend/access/external/hd_work_mgr.c ---
@@ -768,6 +769,16 @@ make_allocation_output_string(List *segment_fragments)
initStringInfo(&segwork);
appendStringInfoString(&segwork, SEGWORK_PREFIX);
+ if (enable_secure_filesystem)
--- End diff --
if we append the dfs_address through the external framework, why do we need to append it here as well? wouldn't it work for both readable and writable tables?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by GodenYao <gi...@git.apache.org>.
Github user GodenYao commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57396779
--- Diff: src/backend/access/external/fileam.c ---
@@ -513,7 +515,7 @@ external_getnext(FileScanDesc scan, ScanDirection direction)
* this function to initialize our various structures and state..
*/
ExternalInsertDesc
-external_insert_init(Relation rel, int errAosegno)
+external_insert_init(Relation rel, int errAosegno, char *namespace)
--- End diff --
maybe add comments to explain why we need namespace here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57266957
--- Diff: src/backend/gpopt/ivy.xml ---
@@ -38,7 +38,7 @@ under the License.
</configurations>
<dependencies>
- <dependency org="emc" name="optimizer" rev="1.623" conf="osx106_x86->osx106_x86_32;osx106_x86_32->osx106_x86_32;rhel5_x86_64->rhel5_x86_64;suse10_x86_64->suse10_x86_64" />
+ <dependency org="emc" name="optimizer" rev="1.625" conf="osx106_x86->osx106_x86_32;osx106_x86_32->osx106_x86_32;rhel5_x86_64->rhel5_x86_64;suse10_x86_64->suse10_x86_64" />
--- End diff --
relevant to this commit?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by kavinderd <gi...@git.apache.org>.
GitHub user kavinderd reopened a pull request:
https://github.com/apache/incubator-hawq/pull/497
Writable security fix
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kavinderd/incubator-hawq writable_security_fix
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/497.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 #497
----
commit 50926e7470e4d5a6c6857d57063f8b8f5634d6f8
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-03-04T18:49:13Z
HAWQ-462. Adding dfs_address from pgfilespace along with fragment data sent to segments
commit c5d3e4a3ba232f69b217f112c9908369d2f2ce2e
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-03-04T21:41:42Z
HAWQ-462. CR fixes
commit f996eb8a69a565ac8f7afd202247bf27ae92c0ff
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-03-08T19:48:58Z
HAWQ-462. Moved dfs address from fragment to segment metadata
commit 07d31d33f8f7cebf735a1d9599a104c0f55daf03
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-03-08T20:03:55Z
HAWQ-462. Fix compilation error
commit 287a7f0aa25fa806c259b7c7a68cc8c89c78045d
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-03-08T22:02:22Z
HAWQ-462. Configure dfs_address only if secure
commit 49dfdd87a91bf046c8c859bfaf12c7d26c19445c
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-03-09T18:49:24Z
HAWQ-462. CR fixes
commit c0d484e16bd27777780de5b71e38e129faea34f0
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-03-17T23:54:37Z
Merge branch 'HAWQ-462' of https://github.com/apache/incubator-hawq into HAWQ-462
commit 55147857cf6cd7ea6552caea24920b58f95a841d
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-03-23T03:43:08Z
Work in progress, pass namespace to segment and eventually to pxf
commit dd3da9152b43d105050da5743ce57fcb8920598c
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-03-23T21:40:49Z
fixes for memory allocation issue
commit 8a95f8e5ea139bda9f58b3b23fc9622c3c35d1a6
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-03-23T21:54:23Z
cleanup
commit e88cb5334db62fc993dbbbed454138e7b98165c6
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-03-23T21:57:20Z
Merge with HAWQ-462
commit 5cb423d500201abfeefe05812116737477f94149
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-03-23T22:48:35Z
fix to passing char by ref
commit 4a4c460d626560cf0caa580a18acd5827c666b15
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-03-23T23:07:32Z
Remove unecessary fields
commit 3d33f49f12b2dfc25a8802af42dc78ccfa78af15
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-03-23T23:09:56Z
Use const for delim
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57267862
--- Diff: src/backend/cdb/cdbquerycontextdispatching.c ---
@@ -102,7 +102,7 @@ int QueryContextDispatchingSizeMemoryLimit = 100 * 1024; /* KB */
enum QueryContextDispatchingItemType
{
- MasterXid, TablespaceLocation, TupleType, EmptyTable, FileSystemCredential
+ MasterXid, TablespaceLocation, TupleType, EmptyTable, FileSystemCredential, Namespace
--- End diff --
can you please explain this part? does it mean the namespace becomes part of the dispatched data? if so, could we potentially get it without changing the external table URL?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd commented on the pull request:
https://github.com/apache/incubator-hawq/pull/497#issuecomment-200588159
@hornn please take a look
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57266801
--- Diff: src/backend/cdb/cdbquerycontextdispatching.c ---
@@ -616,6 +632,37 @@ RebuildFilesystemCredentials(QueryContextInfo *cxt, HTAB ** currentFilesystemCre
pfree(binary);
}
+
+/*
+ * deserialize the namespace data
+ */
+
--- End diff --
remove new line
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by GodenYao <gi...@git.apache.org>.
Github user GodenYao commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57397279
--- Diff: src/bin/gpfusion/gpbridgeapi.c ---
@@ -508,17 +531,14 @@ size_t fill_buffer(gphadoop_context* context, char* start, size_t size)
void add_delegation_token(PxfInputData *inputData)
--- End diff --
probably should clean this parameter as it's not used in the fuction
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57267537
--- Diff: src/backend/cdb/cdbquerycontextdispatching.c ---
@@ -616,6 +632,37 @@ RebuildFilesystemCredentials(QueryContextInfo *cxt, HTAB ** currentFilesystemCre
pfree(binary);
}
+
+/*
+ * deserialize the namespace data
+ */
+
+static void
+RebuildNamespace(QueryContextInfo *cxt)
+{
+
+ int len;
+ char buffer[4], *binary;
+ ReadData(cxt, buffer, sizeof(buffer), TRUE);
+
+ len = (int) ntohl(*(uint32 *) buffer);
+ binary = palloc(len);
+ if(ReadData(cxt, binary, len, TRUE))
+ {
+ StringInfoData buffer;
+ initStringInfoOfString(&buffer, binary, len);
+
+ cxt->sharedPath = palloc(buffer.len + 1);
+ while(buffer.cursor < buffer.len) {
--- End diff --
missing space after while.
Also I think the palloc + loop can be replaced by a copy function `pnstrdup`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57322366
--- Diff: src/backend/access/external/fileam.c ---
@@ -681,6 +687,8 @@ external_insert(ExternalInsertDesc extInsertDesc, HeapTuple instup)
fcinfo.arg[0] = HeapTupleGetDatum(instup);
fcinfo.argnull[0] = false;
+ fcinfo.arg[1] = "test";
--- End diff --
Remnants of when I was prototyping the solution. Will remove.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57322304
--- Diff: src/bin/gpupgrade/crc32c.c ---
@@ -0,0 +1 @@
+../../../src/backend/utils/hash/crc32c.c
--- End diff --
They're not. They accidentally got commited. Will remove
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57322156
--- Diff: src/include/access/fileam.h ---
@@ -44,12 +44,13 @@
* ExternalInsertDescData is used for storing state related
* to inserting data into a writable external table.
*/
-typedef struct ExternalInsertDescData
+typedef struct ExternalInsertDescDat
--- End diff --
oops
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: Writable security fix
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/497#discussion_r57267369
--- Diff: src/backend/access/external/fileam.c ---
@@ -681,6 +687,8 @@ external_insert(ExternalInsertDesc extInsertDesc, HeapTuple instup)
fcinfo.arg[0] = HeapTupleGetDatum(instup);
fcinfo.argnull[0] = false;
+ fcinfo.arg[1] = "test";
--- End diff --
what does it mean?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---