[ 
https://issues.apache.org/jira/browse/PHOENIX-1311?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15193856#comment-15193856
 ] 

Samarth Jain edited comment on PHOENIX-1311 at 3/14/16 6:37 PM:
----------------------------------------------------------------

Thanks for the patch, [~ankit.singhal]. Here is some initial feedback (more to 
come):

1) The new test classes, NamespaceSchemaMappingIT and UseSchemaIT, that you 
have added don't need to extend BaseClientManagedTimeIT since I don't see your 
tests dependent on the connection SCN timestamp. It is always better for tests 
to extend BaseHBaseManagedTimeIT instead.

2) Remove commented code in NamespaceSchemaMappingIT. 

3) Modify the catch block in UseSchemaIT#testUseSchema() and get rid of 
printing stacktrace.

4) Is this change needed in IndexIT.java? 

{code}
props.put(QueryServices.IS_NAMESPACE_MAPPING_ENABLED, Boolean.toString(true));
{code}

5) In CreateSchemaCompiler#compile(), get rid of connectionToBe variable as I 
don't think you need it.

{code}
public MutationPlan compile(final CreateSchemaStatement create) throws 
SQLException {
+        final PhoenixConnection connection = statement.getConnection();
+        PhoenixConnection connectionToBe = connection; 
{code}

6) Make sure your code is formatted and coding guidelines followed. I see wrong 
indentation, missing spaces, etc. in several places.


was (Author: samarthjain):
Thanks for the patch, [~ankit.singhal]. Here is some initial feedback (more to 
come):

1) The new test classes, NamespaceSchemaMappingIT and UseSchemaIT, that you 
have added don't need to extend BaseClientManagedTimeIT since I don't see your 
tests dependent on the connection SCN timestamp. It is always better for tests 
to extend BaseHBaseManagedTimeIT instead.

2) Remove commented code in NamespaceSchemaMappingIT. 

3) Modify the catch block in UseSchemaIT#testUseSchema() and get rid of 
printing stacktrace.

4) Is this change needed in IndexIT.java? 

{code}
props.put(QueryServices.IS_NAMESPACE_MAPPING_ENABLED, Boolean.toString(true));
{code}

5) In CreateSchemaCompiler#compile(), get rid of connectionToBe variable as I 
don't think you need it.

{codE}
public MutationPlan compile(final CreateSchemaStatement create) throws 
SQLException {
+        final PhoenixConnection connection = statement.getConnection();
+        PhoenixConnection connectionToBe = connection; 
{code}

6) Make sure your code is formatted and coding guidelines followed. I see wrong 
indentation, missing spaces, etc. in several places.

> HBase namespaces surfaced in phoenix
> ------------------------------------
>
>                 Key: PHOENIX-1311
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-1311
>             Project: Phoenix
>          Issue Type: New Feature
>            Reporter: nicolas maillard
>            Assignee: Ankit Singhal
>            Priority: Minor
>             Fix For: 4.8.0
>
>         Attachments: PHOENIX-1311.docx, PHOENIX-1311_v1.patch, 
> PHOENIX-1311_wip.patch, PHOENIX-1311_wip_2.patch
>
>
> Hbase (HBASE-8015) has the concept of namespaces in the form of 
> myNamespace:MyTable it would be great if Phoenix leveraged this feature to 
> give a database like feature on top of the table.
> Maybe to stay close to Hbase it could also be a create DB:Table...
> or DB.Table which is a more standard annotation?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to