You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hawq.apache.org by hornn <gi...@git.apache.org> on 2015/12/21 22:57:15 UTC
[GitHub] incubator-hawq pull request: Hawq 257
GitHub user hornn opened a pull request:
https://github.com/apache/incubator-hawq/pull/206
Hawq 257
When parsing PXF URI, change the port to pxf_service_port only if pxf_isilon is ON.
The parsed port is always used (both Isilon and regular cases), which makes the code simpler.
In case of HA, the port is not specified in the URI, so we use pxf_service_port instead.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hornn/incubator-hawq HAWQ-257
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/206.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 #206
----
commit 3d4dd8778d7c568d81135c4a12b4a82956f3db0d
Author: Noa Horn <nh...@pivotal.io>
Date: 2015-12-16T17:10:00Z
HAWQ-257. Remove duplicate definition of pxf_service_port guc
commit baf48b11f446191a60c23a17f751e76b85d94471
Author: Noa Horn <nh...@pivotal.io>
Date: 2015-12-18T19:01:27Z
HAWQ-257. Use user defined port in PXF queries.
pxf_service_port should only be used in HA and Isilon cases.
commit 3664931aa2ff93c7976113a633a359b30f88fb4c
Author: Noa Horn <nh...@pivotal.io>
Date: 2015-12-18T22:29:00Z
HAWQ-257. Fix condition for writable
commit 54aff704a11ce7c1190d9e252d77c7e98a9e8323
Author: Noa Horn <nh...@pivotal.io>
Date: 2015-12-21T19:57:41Z
HAWQ-257. Refactoring and unittesting
----
---
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: Hawq 257
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on the pull request:
https://github.com/apache/incubator-hawq/pull/206#issuecomment-166467715
+!
---
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: Hawq 257
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/206#discussion_r48209738
--- Diff: src/backend/access/external/test/pxfuriparser_test.c ---
@@ -76,6 +77,87 @@ test__parseGPHDUri__ValidURI(void **state)
}
/*
+ * Test parsing of valid uri with with nameservice instead of host and port
+ * as given in LOCATION in a PXF external table.
+ */
+void
+test__parseGPHDUri__ValidURI_HA(void **state)
+{
+ char* uri = "pxf://hanameservice/some/path/and/table.tbl?FRAGMENTER=SomeFragmenter&ACCESSOR=SomeAccessor&RESOLVER=SomeResolver&ANALYZER=SomeAnalyzer";
+ List* options = NIL;
+ ListCell* cell = NULL;
+ OptionData* option = NULL;
+
+ /* mock GPHD_HA_load_nodes */
+ NNHAConf* ha_conf = (NNHAConf *)palloc0(sizeof(NNHAConf));
+ ha_conf->nameservice = "hanameservice";
+ ha_conf->numn = 2;
+ ha_conf->nodes = ((char**)palloc0(sizeof(char*) * 2));
+ ha_conf->nodes[0] = "node1";
+ ha_conf->restports = ((char**)palloc0(sizeof(char*) * 2));
+ ha_conf->restports[0] = "1001";
+ expect_string(GPHD_HA_load_nodes, nameservice, "hanameservice");
+ will_return(GPHD_HA_load_nodes, ha_conf);
+
+ /* mode GPHD_HA_release_nodes */
+ expect_value(GPHD_HA_release_nodes, conf, ha_conf);
+ will_be_called(GPHD_HA_release_nodes);
+
+ GPHDUri* parsed = parseGPHDUri(uri);
+
+ assert_true(parsed != NULL);
+ assert_string_equal(parsed->uri, uri);
+
+ assert_string_equal(parsed->protocol, "pxf");
+ assert_string_equal(parsed->host, "node1"); /* value should be taken from ha_nodes */
+ assert_string_equal(parsed->port, "1001"); /* it should be taken from ha_nodes */
+ assert_false(parsed->ha_nodes == NULL);
+ assert_string_equal(parsed->data, "some/path/and/table.tbl");
+
+ freeGPHDUri(parsed);
+
+ /* free NNHAConf */
+ if (ha_conf)
+ {
+ pfree(ha_conf->nodes);
+ pfree(ha_conf->restports);
+ pfree(ha_conf);
+ }
+}
+
+/*
+ * Test parsing of valid uri as given in LOCATION in a PXF external table,
+ * with pxf_isilon set to true.
+ */
+void
+test__parseGPHDUri__ValidURI_Isilon(void **state)
+{
+ char* uri = "pxf://servername:5000/some/path/and/table.tbl?FRAGMENTER=SomeFragmenter&ACCESSOR=SomeAccessor&RESOLVER=SomeResolver&ANALYZER=SomeAnalyzer";
+ List* options = NIL;
+ ListCell* cell = NULL;
+ OptionData* option = NULL;
+
+ /* set pxf_isilon to true */
+ pxf_isilon = true;
+
+ GPHDUri* parsed = parseGPHDUri(uri);
+
+ assert_true(parsed != NULL);
+ assert_string_equal(parsed->uri, uri);
+
+ assert_string_equal(parsed->protocol, "pxf");
+ assert_string_equal(parsed->host, "servername");
+ assert_int_equal(atoi(parsed->port), pxf_service_port); /* it should be pxf_service_port */
+ assert_true(parsed->ha_nodes == NULL);
+ assert_string_equal(parsed->data, "some/path/and/table.tbl");
+
+ freeGPHDUri(parsed);
+
+ /* set pxf_isilon back to false */
+ pxf_isilon = false;
--- End diff --
is this required ?
---
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: Hawq 257
Posted by hornn <gi...@git.apache.org>.
Github user hornn closed the pull request at:
https://github.com/apache/incubator-hawq/pull/206
---
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: Hawq 257
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/206#discussion_r48211299
--- Diff: src/backend/access/external/test/pxfuriparser_test.c ---
@@ -76,6 +77,87 @@ test__parseGPHDUri__ValidURI(void **state)
}
/*
+ * Test parsing of valid uri with with nameservice instead of host and port
+ * as given in LOCATION in a PXF external table.
+ */
+void
+test__parseGPHDUri__ValidURI_HA(void **state)
+{
+ char* uri = "pxf://hanameservice/some/path/and/table.tbl?FRAGMENTER=SomeFragmenter&ACCESSOR=SomeAccessor&RESOLVER=SomeResolver&ANALYZER=SomeAnalyzer";
+ List* options = NIL;
+ ListCell* cell = NULL;
+ OptionData* option = NULL;
+
+ /* mock GPHD_HA_load_nodes */
+ NNHAConf* ha_conf = (NNHAConf *)palloc0(sizeof(NNHAConf));
+ ha_conf->nameservice = "hanameservice";
+ ha_conf->numn = 2;
+ ha_conf->nodes = ((char**)palloc0(sizeof(char*) * 2));
+ ha_conf->nodes[0] = "node1";
+ ha_conf->restports = ((char**)palloc0(sizeof(char*) * 2));
+ ha_conf->restports[0] = "1001";
+ expect_string(GPHD_HA_load_nodes, nameservice, "hanameservice");
+ will_return(GPHD_HA_load_nodes, ha_conf);
+
+ /* mode GPHD_HA_release_nodes */
+ expect_value(GPHD_HA_release_nodes, conf, ha_conf);
+ will_be_called(GPHD_HA_release_nodes);
+
+ GPHDUri* parsed = parseGPHDUri(uri);
+
+ assert_true(parsed != NULL);
+ assert_string_equal(parsed->uri, uri);
+
+ assert_string_equal(parsed->protocol, "pxf");
+ assert_string_equal(parsed->host, "node1"); /* value should be taken from ha_nodes */
+ assert_string_equal(parsed->port, "1001"); /* it should be taken from ha_nodes */
+ assert_false(parsed->ha_nodes == NULL);
+ assert_string_equal(parsed->data, "some/path/and/table.tbl");
+
+ freeGPHDUri(parsed);
+
+ /* free NNHAConf */
+ if (ha_conf)
+ {
+ pfree(ha_conf->nodes);
+ pfree(ha_conf->restports);
+ pfree(ha_conf);
+ }
+}
+
+/*
+ * Test parsing of valid uri as given in LOCATION in a PXF external table,
+ * with pxf_isilon set to true.
+ */
+void
+test__parseGPHDUri__ValidURI_Isilon(void **state)
+{
+ char* uri = "pxf://servername:5000/some/path/and/table.tbl?FRAGMENTER=SomeFragmenter&ACCESSOR=SomeAccessor&RESOLVER=SomeResolver&ANALYZER=SomeAnalyzer";
+ List* options = NIL;
+ ListCell* cell = NULL;
+ OptionData* option = NULL;
+
+ /* set pxf_isilon to true */
+ pxf_isilon = true;
+
+ GPHDUri* parsed = parseGPHDUri(uri);
+
+ assert_true(parsed != NULL);
+ assert_string_equal(parsed->uri, uri);
+
+ assert_string_equal(parsed->protocol, "pxf");
+ assert_string_equal(parsed->host, "servername");
+ assert_int_equal(atoi(parsed->port), pxf_service_port); /* it should be pxf_service_port */
+ assert_true(parsed->ha_nodes == NULL);
+ assert_string_equal(parsed->data, "some/path/and/table.tbl");
+
+ freeGPHDUri(parsed);
+
+ /* set pxf_isilon back to false */
+ pxf_isilon = false;
--- End diff --
It's a system variable, so if I change it in the beginning of the test (line 141) I should clean after me.
It can potentially affect other tests that assume it has specific value, although that's not very likely.
---
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.
---