You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hawq.apache.org by sansanichfb <gi...@git.apache.org> on 2016/07/19 23:50:06 UTC
[GitHub] incubator-hawq pull request #803: HAWQ-932. Added pxf service address to cur...
GitHub user sansanichfb opened a pull request:
https://github.com/apache/incubator-hawq/pull/803
HAWQ-932. Added pxf service address to curl resolution.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-hawq HAWQ-932
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/803.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 #803
----
commit 66e482120f880d915e26187c37f6173cb768ba75
Author: Oleksandr Diachenko <od...@pivotal.io>
Date: 2016-07-19T23:46:43Z
HAWQ-932. Added pxf service address to curl resolution.
----
---
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 #803: HAWQ-932. Added pxf service address to cur...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-hawq/pull/803
---
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 issue #803: HAWQ-932. Added pxf service address to curl resol...
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd commented on the issue:
https://github.com/apache/incubator-hawq/pull/803
LGTM. Agree with @GodenYao on refactoring out the common code
---
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 #803: HAWQ-932. Added pxf service address to cur...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/803#discussion_r71760909
--- Diff: src/backend/access/external/libchurl.c ---
@@ -312,6 +312,14 @@ CHURL_HANDLE churl_init_upload(const char* url, CHURL_HEADERS headers)
context->upload = true;
clear_error_buffer(context);
+ /* needed to resolve pxf service address */
+ struct curl_slist *resolve_hosts = NULL;
+ char *pxf_host_entry = (char *) palloc0(strlen(pxf_service_address) + strlen(LocalhostIpV4Entry) + 1);
+ strcat(pxf_host_entry, pxf_service_address);
--- End diff --
Yes. I was suggesting that we add the below OPT only when pxf_service_address is not based on IP address
---
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 #803: HAWQ-932. Added pxf service address to cur...
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/803#discussion_r71555148
--- Diff: src/backend/access/external/libchurl.c ---
@@ -312,6 +312,14 @@ CHURL_HANDLE churl_init_upload(const char* url, CHURL_HEADERS headers)
context->upload = true;
clear_error_buffer(context);
+ /* needed to resolve pxf service address */
+ struct curl_slist *resolve_hosts = NULL;
+ char *pxf_host_entry = (char *) palloc0(strlen(pxf_service_address) + strlen(LocalhostIpV4Entry) + 1);
--- End diff --
Is `pxf_host_entry` pfree'd?
---
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 #803: HAWQ-932. Added pxf service address to cur...
Posted by sansanichfb <gi...@git.apache.org>.
Github user sansanichfb commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/803#discussion_r71808630
--- Diff: src/backend/catalog/external/externalmd.c ---
@@ -84,7 +84,7 @@ List *ParsePxfEntries(StringInfo json, char *profile, Oid dboid)
{
struct json_object *jsonItem = json_object_array_get_idx(jsonItems, i);
PxfItem *pxfItem = ParsePxfItem(jsonItem, profile);
- if (dboid != NULL)
+ if (dboid != InvalidOid)
--- End diff --
Warning.
---
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 #803: HAWQ-932. Added pxf service address to cur...
Posted by sansanichfb <gi...@git.apache.org>.
Github user sansanichfb commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/803#discussion_r71808574
--- Diff: src/backend/utils/adt/pxf_functions.c ---
@@ -19,6 +19,7 @@
#include "postgres.h"
+#include "access/hd_work_mgr.h"
--- End diff --
Without this fix I was getting:
```
pxf_functions.c:46:10: warning: implicit declaration of function 'get_pxf_item_metadata' is invalid in C99
[-Wimplicit-function-declaration]
items = get_pxf_item_metadata(profile_cstr, pattern_cstr, NULL);
^
pxf_functions.c:46:8: warning: incompatible integer to pointer conversion assigning to 'List *' (aka 'struct List *') from 'int'
[-Wint-conversion]
items = get_pxf_item_metadata(profile_cstr, pattern_cstr, NULL);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.
```
---
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 #803: HAWQ-932. Added pxf service address to cur...
Posted by sansanichfb <gi...@git.apache.org>.
Github user sansanichfb commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/803#discussion_r71808615
--- Diff: src/backend/utils/adt/pxf_functions.c ---
@@ -43,7 +44,7 @@ pxf_item_fields_enum_start(text *profile, text *pattern)
char *profile_cstr = text_to_cstring(profile);
char *pattern_cstr = text_to_cstring(pattern);
- items = get_pxf_item_metadata(profile_cstr, pattern_cstr, NULL);
+ items = get_pxf_item_metadata(profile_cstr, pattern_cstr, InvalidOid);
--- End diff --
One more warning.
---
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 #803: HAWQ-932. Added pxf service address to cur...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/803#discussion_r71760169
--- Diff: src/backend/access/external/libchurl.c ---
@@ -312,6 +312,14 @@ CHURL_HANDLE churl_init_upload(const char* url, CHURL_HEADERS headers)
context->upload = true;
clear_error_buffer(context);
+ /* needed to resolve pxf service address */
+ struct curl_slist *resolve_hosts = NULL;
+ char *pxf_host_entry = (char *) palloc0(strlen(pxf_service_address) + strlen(LocalhostIpV4Entry) + 1);
+ strcat(pxf_host_entry, pxf_service_address);
--- End diff --
This step might be unnecssary if the pxf_service_address itself is based on the IP address
---
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 issue #803: HAWQ-932. Added pxf service address to curl resol...
Posted by GodenYao <gi...@git.apache.org>.
Github user GodenYao commented on the issue:
https://github.com/apache/incubator-hawq/pull/803
churl_init_upload and churl_init_download only has 1 tag difference ( context->upload = true vs. false) probably worth refactoring to a single function and invoked by these 2, which will make it semantic readable as well as easy to maintain
---
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 issue #803: HAWQ-932. Added pxf service address to curl resol...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on the issue:
https://github.com/apache/incubator-hawq/pull/803
+1
---
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 #803: HAWQ-932. Added pxf service address to cur...
Posted by sansanichfb <gi...@git.apache.org>.
Github user sansanichfb commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/803#discussion_r71760410
--- Diff: src/backend/access/external/libchurl.c ---
@@ -312,6 +312,14 @@ CHURL_HANDLE churl_init_upload(const char* url, CHURL_HEADERS headers)
context->upload = true;
clear_error_buffer(context);
+ /* needed to resolve pxf service address */
+ struct curl_slist *resolve_hosts = NULL;
+ char *pxf_host_entry = (char *) palloc0(strlen(pxf_service_address) + strlen(LocalhostIpV4Entry) + 1);
+ strcat(pxf_host_entry, pxf_service_address);
--- End diff --
For case when user created external table referring to "localhost" it's 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 #803: HAWQ-932. Added pxf service address to cur...
Posted by sansanichfb <gi...@git.apache.org>.
Github user sansanichfb commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/803#discussion_r71808647
--- Diff: src/backend/access/external/pxfmasterapi.c ---
@@ -434,5 +434,5 @@ List* get_and_cache_external_metadata(GPHDUri* hadoop_uri, char *profile, char *
List* get_no_cache_external_metadata(GPHDUri* hadoop_uri, char *profile, char *pattern, ClientContext *client_context)
{
- return get_external_metadata(hadoop_uri, profile, pattern, client_context, NULL);
+ return get_external_metadata(hadoop_uri, profile, pattern, client_context, InvalidOid);
--- End diff --
Fixed warning.
---
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.
---