You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2022/03/08 20:36:13 UTC
[GitHub] [hive] scarlin-cloudera opened a new pull request #3084: HIVE-26015: URLEncode the URI for Ranger auth for HBase
scarlin-cloudera opened a new pull request #3084:
URL: https://github.com/apache/hive/pull/3084
Fields in the Serde Properties can have a hash tag in it,
so the URI needs to be URLEncoded.
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
We need to URL Encode the Serde column properties for HBase because it is converted into a URI
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
The SerDe Properties have a "#" character which needs to be URL Encoded in order to be passed to Ranger.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
Tested on my private environment using a Create table statement. Too complicated to create a unit test for this fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] scarlin-cloudera commented on a change in pull request #3084: HIVE-26015: URLEncode the URI for Ranger auth for HBase
Posted by GitBox <gi...@apache.org>.
scarlin-cloudera commented on a change in pull request #3084:
URL: https://github.com/apache/hive/pull/3084#discussion_r823364267
##########
File path: hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseStorageHandler.java
##########
@@ -296,7 +297,8 @@ public URI getURIForAuth(Table table) throws URISyntaxException {
String hbase_host = tableProperties.containsKey(HBASE_HOST_NAME)? tableProperties.get(HBASE_HOST_NAME) : hbaseConf.get(HBASE_HOST_NAME);
String hbase_port = tableProperties.containsKey(HBASE_CLIENT_PORT)? tableProperties.get(HBASE_CLIENT_PORT) : hbaseConf.get(HBASE_CLIENT_PORT);
String table_name = tableProperties.getOrDefault(HBaseSerDe.HBASE_TABLE_NAME, null);
Review comment:
So I was thinking about doing it for the whole URI and not just the properties.
I didn't research the characters that get URL Encoded versus the characters that are used in a table, but I was a bit nervous to URL Encode the table name which might break backward compatibility.
I figured URL encoding Serde properties was pretty safe though since it probably isn't used by Ranger. I also thought about removing Serde properties from the URI, but that also had me worried about backward compatibility.
I'm open to changing this or having more of a discussion on it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] scarlin-cloudera commented on a change in pull request #3084: HIVE-26015: URLEncode the URI for Ranger auth for HBase
Posted by GitBox <gi...@apache.org>.
scarlin-cloudera commented on a change in pull request #3084:
URL: https://github.com/apache/hive/pull/3084#discussion_r824094639
##########
File path: hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseStorageHandler.java
##########
@@ -296,7 +297,8 @@ public URI getURIForAuth(Table table) throws URISyntaxException {
String hbase_host = tableProperties.containsKey(HBASE_HOST_NAME)? tableProperties.get(HBASE_HOST_NAME) : hbaseConf.get(HBASE_HOST_NAME);
String hbase_port = tableProperties.containsKey(HBASE_CLIENT_PORT)? tableProperties.get(HBASE_CLIENT_PORT) : hbaseConf.get(HBASE_CLIENT_PORT);
String table_name = tableProperties.getOrDefault(HBaseSerDe.HBASE_TABLE_NAME, null);
Review comment:
Changed it so we're URLEncoding the whole URI now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] scarlin-cloudera commented on a change in pull request #3084: HIVE-26015: URLEncode the URI for Ranger auth for HBase
Posted by GitBox <gi...@apache.org>.
scarlin-cloudera commented on a change in pull request #3084:
URL: https://github.com/apache/hive/pull/3084#discussion_r824095006
##########
File path: hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseStorageHandler.java
##########
@@ -296,7 +297,8 @@ public URI getURIForAuth(Table table) throws URISyntaxException {
String hbase_host = tableProperties.containsKey(HBASE_HOST_NAME)? tableProperties.get(HBASE_HOST_NAME) : hbaseConf.get(HBASE_HOST_NAME);
String hbase_port = tableProperties.containsKey(HBASE_CLIENT_PORT)? tableProperties.get(HBASE_CLIENT_PORT) : hbaseConf.get(HBASE_CLIENT_PORT);
String table_name = tableProperties.getOrDefault(HBaseSerDe.HBASE_TABLE_NAME, null);
- String column_family = tableProperties.getOrDefault(HBaseSerDe.HBASE_COLUMNS_MAPPING, null);
+ String column_family =
+ URLEncoder.encode(tableProperties.getOrDefault(HBaseSerDe.HBASE_COLUMNS_MAPPING, null));
if (column_family != null)
return new URI(HBASE_PREFIX+"//"+hbase_host+":"+hbase_port+"/"+table_name+"/"+column_family);
Review comment:
Changed the code so we re resolving the whole URI now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] scarlin-cloudera commented on a change in pull request #3084: HIVE-26015: URLEncode the URI for Ranger auth for HBase
Posted by GitBox <gi...@apache.org>.
scarlin-cloudera commented on a change in pull request #3084:
URL: https://github.com/apache/hive/pull/3084#discussion_r823365104
##########
File path: hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseStorageHandler.java
##########
@@ -296,7 +297,8 @@ public URI getURIForAuth(Table table) throws URISyntaxException {
String hbase_host = tableProperties.containsKey(HBASE_HOST_NAME)? tableProperties.get(HBASE_HOST_NAME) : hbaseConf.get(HBASE_HOST_NAME);
String hbase_port = tableProperties.containsKey(HBASE_CLIENT_PORT)? tableProperties.get(HBASE_CLIENT_PORT) : hbaseConf.get(HBASE_CLIENT_PORT);
String table_name = tableProperties.getOrDefault(HBaseSerDe.HBASE_TABLE_NAME, null);
- String column_family = tableProperties.getOrDefault(HBaseSerDe.HBASE_COLUMNS_MAPPING, null);
+ String column_family =
+ URLEncoder.encode(tableProperties.getOrDefault(HBaseSerDe.HBASE_COLUMNS_MAPPING, null));
if (column_family != null)
return new URI(HBASE_PREFIX+"//"+hbase_host+":"+hbase_port+"/"+table_name+"/"+column_family);
Review comment:
Oh, I just wrote the whole comment above addressing this before I saw yours. Please read what I wrote above.
Perhaps it is better to encode the whole string? I just was worried about backward compatibility and the potential for URL Encoding characters that might get by in the URI...For instance, we found that one "#" characters worked fine in Serde properties, but we only hit the problem when there were 2 '#' characters. I'm a bit worried about breaking something.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak closed pull request #3084: HIVE-26015: URLEncode the URI for Ranger auth for HBase
Posted by GitBox <gi...@apache.org>.
zabetak closed pull request #3084:
URL: https://github.com/apache/hive/pull/3084
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak commented on a change in pull request #3084: HIVE-26015: URLEncode the URI for Ranger auth for HBase
Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #3084:
URL: https://github.com/apache/hive/pull/3084#discussion_r822541179
##########
File path: hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseStorageHandler.java
##########
@@ -296,7 +297,8 @@ public URI getURIForAuth(Table table) throws URISyntaxException {
String hbase_host = tableProperties.containsKey(HBASE_HOST_NAME)? tableProperties.get(HBASE_HOST_NAME) : hbaseConf.get(HBASE_HOST_NAME);
String hbase_port = tableProperties.containsKey(HBASE_CLIENT_PORT)? tableProperties.get(HBASE_CLIENT_PORT) : hbaseConf.get(HBASE_CLIENT_PORT);
String table_name = tableProperties.getOrDefault(HBaseSerDe.HBASE_TABLE_NAME, null);
- String column_family = tableProperties.getOrDefault(HBaseSerDe.HBASE_COLUMNS_MAPPING, null);
+ String column_family =
+ URLEncoder.encode(tableProperties.getOrDefault(HBaseSerDe.HBASE_COLUMNS_MAPPING, null));
if (column_family != null)
return new URI(HBASE_PREFIX+"//"+hbase_host+":"+hbase_port+"/"+table_name+"/"+column_family);
Review comment:
Why don't we encode the whole string before passing to the URI constructor?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] asolimando commented on pull request #3084: HIVE-26015: URLEncode the URI for Ranger auth for HBase
Posted by GitBox <gi...@apache.org>.
asolimando commented on pull request #3084:
URL: https://github.com/apache/hive/pull/3084#issuecomment-1066884312
LGTM @scarlin-cloudera, I saw a single test failing, which is failing for one other PR of mine deterministically (it looks like broken for good, not a flaky test), let's see if we can track down which commit broke this and what to do to fix it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] asolimando commented on a change in pull request #3084: HIVE-26015: URLEncode the URI for Ranger auth for HBase
Posted by GitBox <gi...@apache.org>.
asolimando commented on a change in pull request #3084:
URL: https://github.com/apache/hive/pull/3084#discussion_r822446138
##########
File path: hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseStorageHandler.java
##########
@@ -296,7 +297,8 @@ public URI getURIForAuth(Table table) throws URISyntaxException {
String hbase_host = tableProperties.containsKey(HBASE_HOST_NAME)? tableProperties.get(HBASE_HOST_NAME) : hbaseConf.get(HBASE_HOST_NAME);
String hbase_port = tableProperties.containsKey(HBASE_CLIENT_PORT)? tableProperties.get(HBASE_CLIENT_PORT) : hbaseConf.get(HBASE_CLIENT_PORT);
String table_name = tableProperties.getOrDefault(HBaseSerDe.HBASE_TABLE_NAME, null);
Review comment:
Is the table name guaranteed not to have symbols which require URL-encoding?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak commented on a change in pull request #3084: HIVE-26015: URLEncode the URI for Ranger auth for HBase
Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #3084:
URL: https://github.com/apache/hive/pull/3084#discussion_r832014955
##########
File path: hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestHBaseStorageHandler.java
##########
@@ -46,6 +53,46 @@ public void testHbaseConfigIsAddedToJobConf() {
jobConfToConfigure.get("hbase.some.fake.option.from.xml.file") != null);
}
+ @Test
+ public void testGetUriForAuth() {
+ try {
+ HBaseStorageHandler hbaseStorageHandler = new HBaseStorageHandler();
+ hbaseStorageHandler.setConf(new JobConf(new HiveConf()));
+ Table table = createMockTable(new HashMap<>());
+ URI uri = hbaseStorageHandler.getURIForAuth(table);
+ // If there is no tablename provided, the default "null" is still
+ // written out. At the time this test was written, this was the current
+ // behavior, so I left this test as/is. Need to research if a null
+ // table can be provided here.
+ Assert.assertEquals("hbase://localhost:2181/null", uri.toString());
+
+ Map<String, String> serdeParams = new HashMap<>();
+ serdeParams.put("hbase.zookeeper.quorum", "testhost");
+ serdeParams.put("hbase.zookeeper.property.clientPort", "8765");
+ table = createMockTable(serdeParams);
+ uri = hbaseStorageHandler.getURIForAuth(table);
+ Assert.assertEquals("hbase://testhost:8765/null", uri.toString());
+
+ serdeParams.put("hbase.table.name", "mytbl");
+ table = createMockTable(serdeParams);
+ uri = hbaseStorageHandler.getURIForAuth(table);
+ Assert.assertEquals("hbase://testhost:8765/mytbl", uri.toString());
+
+ serdeParams.put("hbase.columns.mapping", "mycolumns");
+ table = createMockTable(serdeParams);
+ uri = hbaseStorageHandler.getURIForAuth(table);
+ Assert.assertEquals("hbase://testhost:8765/mytbl/mycolumns", uri.toString());
+
+ serdeParams.put("hbase.table.name", "my#tbl");
+ serdeParams.put("hbase.columns.mapping", "myco#lumns");
+ table = createMockTable(serdeParams);
+ uri = hbaseStorageHandler.getURIForAuth(table);
+ Assert.assertEquals("hbase://testhost:8765/my%23tbl/myco%23lumns", uri.toString());
+ } catch (URISyntaxException e) {
+ throw new RuntimeException(e);
+ }
Review comment:
The catch and rethrow pattern is rarely necessary in unit tests. Moreover, it has few disadvantages such as obscuring the original exception & error message, increasing indentation etc. The try-catch block can be removed and the `URISyntaxException` can go into the declaration of the method if necessary.
##########
File path: hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestHBaseStorageHandler.java
##########
@@ -56,4 +103,16 @@ private TableDesc getHBaseTableDesc() {
Mockito.when(tableDesc.getProperties()).thenReturn(properties);
return tableDesc;
}
+
+ private Table createMockTable(Map<String, String> serdeParams) {
+ Table table = new Table();
+ StorageDescriptor sd = new StorageDescriptor();
+ SerDeInfo sdi = new SerDeInfo();
+ Map<String, String> params = new HashMap<>();
+ sdi.setParameters(serdeParams);
+ sd.setSerdeInfo(sdi);
+ table.setSd(sd);
+ table.setParameters(params);
Review comment:
nit: Change to
`table.setParameters(Collections.emptyMap())` or
`table.setParameters(new HashMap<>())` or
remove entirely if it doesn't call failures
##########
File path: hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseStorageHandler.java
##########
@@ -293,14 +294,23 @@ public void configureTableJobProperties(
public URI getURIForAuth(Table table) throws URISyntaxException {
Map<String, String> tableProperties = HiveCustomStorageHandlerUtils.getTableProperties(table);
hbaseConf = getConf();
- String hbase_host = tableProperties.containsKey(HBASE_HOST_NAME)? tableProperties.get(HBASE_HOST_NAME) : hbaseConf.get(HBASE_HOST_NAME);
- String hbase_port = tableProperties.containsKey(HBASE_CLIENT_PORT)? tableProperties.get(HBASE_CLIENT_PORT) : hbaseConf.get(HBASE_CLIENT_PORT);
- String table_name = tableProperties.getOrDefault(HBaseSerDe.HBASE_TABLE_NAME, null);
- String column_family = tableProperties.getOrDefault(HBaseSerDe.HBASE_COLUMNS_MAPPING, null);
- if (column_family != null)
- return new URI(HBASE_PREFIX+"//"+hbase_host+":"+hbase_port+"/"+table_name+"/"+column_family);
- else
- return new URI(HBASE_PREFIX+"//"+hbase_host+":"+hbase_port+"/"+table_name);
+ String hbase_host = tableProperties.getOrDefault(HBASE_HOST_NAME,
+ hbaseConf.get(HBASE_HOST_NAME));
+ String hbase_port = tableProperties.getOrDefault(HBASE_CLIENT_PORT,
+ hbaseConf.get(HBASE_CLIENT_PORT));
+ String table_name = encodeString(tableProperties.getOrDefault(HBaseSerDe.HBASE_TABLE_NAME,
+ null));
+ String column_family = encodeString(tableProperties.getOrDefault(
+ HBaseSerDe.HBASE_COLUMNS_MAPPING, null));
+ String URIString = HBASE_PREFIX + "//" + hbase_host + ":" + hbase_port + "/" + table_name;
+ if (column_family != null) {
+ URIString += "/" + column_family;
+ }
+ return new URI(URIString);
+ }
+
+ private static String encodeString(String encodedString) {
Review comment:
nit: `encodedString -> rawString`
##########
File path: hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestHBaseStorageHandler.java
##########
@@ -46,6 +53,46 @@ public void testHbaseConfigIsAddedToJobConf() {
jobConfToConfigure.get("hbase.some.fake.option.from.xml.file") != null);
}
+ @Test
+ public void testGetUriForAuth() {
Review comment:
The method seems to contains five tests. They all test `getURIForAuth` method but the input for every test is different. I am a big fan of keeping one assert per test method which tends to make test more readable and maintainable. There are various books (e.g., "The art of unit testing" by Roy Osherove) which go into more detail of why this is a good idea and in this case it seems beneficial to split and refactor the method into five separate tests.
The common test code could go into a method `checkUriForAuth(serdeParams, expectedUri)` and the 5 test methods would call this with a different set of parameters.
##########
File path: hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseStorageHandler.java
##########
@@ -293,14 +294,23 @@ public void configureTableJobProperties(
public URI getURIForAuth(Table table) throws URISyntaxException {
Map<String, String> tableProperties = HiveCustomStorageHandlerUtils.getTableProperties(table);
hbaseConf = getConf();
- String hbase_host = tableProperties.containsKey(HBASE_HOST_NAME)? tableProperties.get(HBASE_HOST_NAME) : hbaseConf.get(HBASE_HOST_NAME);
- String hbase_port = tableProperties.containsKey(HBASE_CLIENT_PORT)? tableProperties.get(HBASE_CLIENT_PORT) : hbaseConf.get(HBASE_CLIENT_PORT);
- String table_name = tableProperties.getOrDefault(HBaseSerDe.HBASE_TABLE_NAME, null);
- String column_family = tableProperties.getOrDefault(HBaseSerDe.HBASE_COLUMNS_MAPPING, null);
- if (column_family != null)
- return new URI(HBASE_PREFIX+"//"+hbase_host+":"+hbase_port+"/"+table_name+"/"+column_family);
- else
- return new URI(HBASE_PREFIX+"//"+hbase_host+":"+hbase_port+"/"+table_name);
+ String hbase_host = tableProperties.getOrDefault(HBASE_HOST_NAME,
+ hbaseConf.get(HBASE_HOST_NAME));
+ String hbase_port = tableProperties.getOrDefault(HBASE_CLIENT_PORT,
+ hbaseConf.get(HBASE_CLIENT_PORT));
+ String table_name = encodeString(tableProperties.getOrDefault(HBaseSerDe.HBASE_TABLE_NAME,
+ null));
+ String column_family = encodeString(tableProperties.getOrDefault(
+ HBaseSerDe.HBASE_COLUMNS_MAPPING, null));
+ String URIString = HBASE_PREFIX + "//" + hbase_host + ":" + hbase_port + "/" + table_name;
+ if (column_family != null) {
+ URIString += "/" + column_family;
+ }
+ return new URI(URIString);
+ }
+
+ private static String encodeString(String encodedString) {
+ return encodedString != null ? URLEncoder.encode(encodedString): null;
Review comment:
Is it safe to rely on the platforms default encoding? Note that URLEncoder.encode(s) is deprecated and the suggested replacement is `URLEncoder.encode(String s, String enc)` with `UTF-8`. In various places in Hive we are using the latter.
##########
File path: hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestHBaseStorageHandler.java
##########
@@ -56,4 +103,16 @@ private TableDesc getHBaseTableDesc() {
Mockito.when(tableDesc.getProperties()).thenReturn(properties);
return tableDesc;
}
+
+ private Table createMockTable(Map<String, String> serdeParams) {
Review comment:
Make static?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org