You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by "Gang Tim Liu (JIRA)" <ji...@apache.org> on 2012/08/17 09:36:37 UTC

[jira] [Created] (HIVE-3394) Refractor a few classes' constructor with builder pattern

Gang Tim Liu created HIVE-3394:
----------------------------------

             Summary: Refractor a few classes' constructor with builder pattern
                 Key: HIVE-3394
                 URL: https://issues.apache.org/jira/browse/HIVE-3394
             Project: Hive
          Issue Type: Bug
         Environment: It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:

1. MStorageDescriptor.java
2. ColumnInfo.java
3. ParseContext.java
4. CreateTableDesc.java
5. ExprNodeColumnDesc.java
            Reporter: Gang Tim Liu
            Assignee: Gang Tim Liu
            Priority: Minor




--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HIVE-3394) Refactor a few classes' constructors with creational patterns

Posted by "Gang Tim Liu (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13437265#comment-13437265 ] 

Gang Tim Liu commented on HIVE-3394:
------------------------------------

@Carl: thank you for giving me the opportunities to work on it. Initially have natural resistance due to work plan but quickly realize it is good thing to do and will make HIVE better which is my goal. I enjoy this refactor with them in mind. have a good weekend.
                
> Refactor a few classes' constructors with creational patterns
> -------------------------------------------------------------
>
>                 Key: HIVE-3394
>                 URL: https://issues.apache.org/jira/browse/HIVE-3394
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Gang Tim Liu
>            Assignee: Gang Tim Liu
>            Priority: Minor
>         Attachments: HIVE-3394.patch.1
>
>
> It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HIVE-3394) Refactor a few classes' constructors with creational patterns

Posted by "Gang Tim Liu (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13438007#comment-13438007 ] 

Gang Tim Liu commented on HIVE-3394:
------------------------------------

@Carl, great! will address them and have a patch ready before noon. thanks
                
> Refactor a few classes' constructors with creational patterns
> -------------------------------------------------------------
>
>                 Key: HIVE-3394
>                 URL: https://issues.apache.org/jira/browse/HIVE-3394
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Gang Tim Liu
>            Assignee: Gang Tim Liu
>            Priority: Minor
>         Attachments: HIVE-3394.patch.1
>
>
> It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (HIVE-3394) Refractor a few classes' constructor with builder pattern

Posted by "Gang Tim Liu (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gang Tim Liu updated HIVE-3394:
-------------------------------

    Description: 
It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:

1. MStorageDescriptor.java
2. ColumnInfo.java
3. ParseContext.java
4. CreateTableDesc.java
5. ExprNodeColumnDesc.java

    Environment:     (was: It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:

1. MStorageDescriptor.java
2. ColumnInfo.java
3. ParseContext.java
4. CreateTableDesc.java
5. ExprNodeColumnDesc.java)
    
> Refractor a few classes' constructor with builder pattern
> ---------------------------------------------------------
>
>                 Key: HIVE-3394
>                 URL: https://issues.apache.org/jira/browse/HIVE-3394
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Gang Tim Liu
>            Assignee: Gang Tim Liu
>            Priority: Minor
>
> It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HIVE-3394) Refractor a few classes' constructor with builder pattern

Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13436622#comment-13436622 ] 

Carl Steinbach commented on HIVE-3394:
--------------------------------------

@Tim: Thanks for following up on this!
                
> Refractor a few classes' constructor with builder pattern
> ---------------------------------------------------------
>
>                 Key: HIVE-3394
>                 URL: https://issues.apache.org/jira/browse/HIVE-3394
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Gang Tim Liu
>            Assignee: Gang Tim Liu
>            Priority: Minor
>
> It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Work started] (HIVE-3394) Refactor a few classes' constructors with creational patterns

Posted by "Gang Tim Liu (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Work on HIVE-3394 started by Gang Tim Liu.

> Refactor a few classes' constructors with creational patterns
> -------------------------------------------------------------
>
>                 Key: HIVE-3394
>                 URL: https://issues.apache.org/jira/browse/HIVE-3394
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Gang Tim Liu
>            Assignee: Gang Tim Liu
>            Priority: Minor
>         Attachments: HIVE-3394.patch.1
>
>
> It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (HIVE-3394) Refactor a few classes' constructors with creational patterns

Posted by "Namit Jain (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Namit Jain updated HIVE-3394:
-----------------------------

    Status: Open  (was: Patch Available)

comments on phabricator
                
> Refactor a few classes' constructors with creational patterns
> -------------------------------------------------------------
>
>                 Key: HIVE-3394
>                 URL: https://issues.apache.org/jira/browse/HIVE-3394
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Gang Tim Liu
>            Assignee: Gang Tim Liu
>            Priority: Minor
>         Attachments: HIVE-3394.patch.1, HIVE-3394.patch.2
>
>
> It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HIVE-3394) Refactor a few classes' constructors with creational patterns

Posted by "Gang Tim Liu (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13437036#comment-13437036 ] 

Gang Tim Liu commented on HIVE-3394:
------------------------------------

https://reviews.facebook.net/D4719
                
> Refactor a few classes' constructors with creational patterns
> -------------------------------------------------------------
>
>                 Key: HIVE-3394
>                 URL: https://issues.apache.org/jira/browse/HIVE-3394
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Gang Tim Liu
>            Assignee: Gang Tim Liu
>            Priority: Minor
>         Attachments: HIVE-3394.patch.1
>
>
> It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (HIVE-3394) Refactor a few classes' constructors with creational patterns

Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Carl Steinbach updated HIVE-3394:
---------------------------------

    Status: Open  (was: Patch Available)
    
> Refactor a few classes' constructors with creational patterns
> -------------------------------------------------------------
>
>                 Key: HIVE-3394
>                 URL: https://issues.apache.org/jira/browse/HIVE-3394
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Gang Tim Liu
>            Assignee: Gang Tim Liu
>            Priority: Minor
>         Attachments: HIVE-3394.patch.1
>
>
> It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HIVE-3394) Refactor a few classes' constructors with creational patterns

Posted by "Gang Tim Liu (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13436996#comment-13436996 ] 

Gang Tim Liu commented on HIVE-3394:
------------------------------------

second thought on MStorageDescriptor.java. It's model class for JDO and let's keep it simple. we can use setter/getter here.
                
> Refactor a few classes' constructors with creational patterns
> -------------------------------------------------------------
>
>                 Key: HIVE-3394
>                 URL: https://issues.apache.org/jira/browse/HIVE-3394
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Gang Tim Liu
>            Assignee: Gang Tim Liu
>            Priority: Minor
>
> It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (HIVE-3394) Refactor a few classes' constructors with creational patterns

Posted by "Gang Tim Liu (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gang Tim Liu updated HIVE-3394:
-------------------------------

    Status: Patch Available  (was: In Progress)

Patch is ready for review in both jira and phabricator.
                
> Refactor a few classes' constructors with creational patterns
> -------------------------------------------------------------
>
>                 Key: HIVE-3394
>                 URL: https://issues.apache.org/jira/browse/HIVE-3394
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Gang Tim Liu
>            Assignee: Gang Tim Liu
>            Priority: Minor
>         Attachments: HIVE-3394.patch.1
>
>
> It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (HIVE-3394) Refactor a few classes' constructors with creational patterns

Posted by "Gang Tim Liu (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gang Tim Liu updated HIVE-3394:
-------------------------------

    Summary: Refactor a few classes' constructors with creational patterns  (was: Refractor a few classes' constructor with builder pattern)
    
> Refactor a few classes' constructors with creational patterns
> -------------------------------------------------------------
>
>                 Key: HIVE-3394
>                 URL: https://issues.apache.org/jira/browse/HIVE-3394
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Gang Tim Liu
>            Assignee: Gang Tim Liu
>            Priority: Minor
>
> It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (HIVE-3394) Refactor a few classes' constructors with creational patterns

Posted by "Gang Tim Liu (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gang Tim Liu updated HIVE-3394:
-------------------------------

    Attachment: HIVE-3394.patch.1
    
> Refactor a few classes' constructors with creational patterns
> -------------------------------------------------------------
>
>                 Key: HIVE-3394
>                 URL: https://issues.apache.org/jira/browse/HIVE-3394
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Gang Tim Liu
>            Assignee: Gang Tim Liu
>            Priority: Minor
>         Attachments: HIVE-3394.patch.1
>
>
> It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Work started] (HIVE-3394) Refactor a few classes' constructors with creational patterns

Posted by "Gang Tim Liu (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Work on HIVE-3394 started by Gang Tim Liu.

> Refactor a few classes' constructors with creational patterns
> -------------------------------------------------------------
>
>                 Key: HIVE-3394
>                 URL: https://issues.apache.org/jira/browse/HIVE-3394
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Gang Tim Liu
>            Assignee: Gang Tim Liu
>            Priority: Minor
>         Attachments: HIVE-3394.patch.1
>
>
> It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HIVE-3394) Refactor a few classes' constructors with creational patterns

Posted by "Gang Tim Liu (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13436969#comment-13436969 ] 

Gang Tim Liu commented on HIVE-3394:
------------------------------------

Problem
=======
Several classes have many parameters in constructor and it doesn't scale well.

Solution
========
After reviewing design patterns (factory method/abstract factory/builder/static factory) in GOF and builder pattern in Joshua's Effective in JAVA (Item #2), feel builder pattern in effective in Java is a good fit since it is designed to solve our problem.

Details on each instance
========================
1. MStorageDescriptor.java
   use builder pattern
2. ColumnInfo.java
   # of parameter is not big yet but the number of references to the constructors is huge: 40+. risk is high to change them all.
   solution is use javabean pattern (setter/getter) for new parameters in hive 3072
3. ParseContext.java
   use builder pattern
4. CreateTableDesc.java
   use builder pattern
5. ExprNodeColumnDesc.java
   same as #2
                
> Refactor a few classes' constructors with creational patterns
> -------------------------------------------------------------
>
>                 Key: HIVE-3394
>                 URL: https://issues.apache.org/jira/browse/HIVE-3394
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Gang Tim Liu
>            Assignee: Gang Tim Liu
>            Priority: Minor
>
> It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (HIVE-3394) Refactor a few classes' constructors with creational patterns

Posted by "Gang Tim Liu (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gang Tim Liu updated HIVE-3394:
-------------------------------

    Status: Patch Available  (was: In Progress)

Patch is available in both jira and phabricator. Some explannations in phabricator.
                
> Refactor a few classes' constructors with creational patterns
> -------------------------------------------------------------
>
>                 Key: HIVE-3394
>                 URL: https://issues.apache.org/jira/browse/HIVE-3394
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Gang Tim Liu
>            Assignee: Gang Tim Liu
>            Priority: Minor
>         Attachments: HIVE-3394.patch.1, HIVE-3394.patch.2
>
>
> It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (HIVE-3394) Refactor a few classes' constructors with creational patterns

Posted by "Gang Tim Liu (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gang Tim Liu updated HIVE-3394:
-------------------------------

    Attachment: HIVE-3394.patch.2
    
> Refactor a few classes' constructors with creational patterns
> -------------------------------------------------------------
>
>                 Key: HIVE-3394
>                 URL: https://issues.apache.org/jira/browse/HIVE-3394
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Gang Tim Liu
>            Assignee: Gang Tim Liu
>            Priority: Minor
>         Attachments: HIVE-3394.patch.1, HIVE-3394.patch.2
>
>
> It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HIVE-3394) Refactor a few classes' constructors with creational patterns

Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HIVE-3394?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13437715#comment-13437715 ] 

Carl Steinbach commented on HIVE-3394:
--------------------------------------

@Tim: Thanks for taking time to look into this.

I recommended using the builder pattern because these constructors are unreadable due to the large number of input parameters. Replacing a twenty-one parameter constructor (i.e. CreateTableDesc()) with a builder object that takes 21 input parameters doesn't really solve the problem. Here's a quote from chapter2 of Bloch's Effective Java book:

{quote}
Instead of making the desired object directly, the client calls a constructor (or static factory) with all of the the *required parameters* and gets a builder object. Then the client calls setter-like methods on the builder object to set each *optional parameter* of interest. Finally, the client calls a parameterless build() method to generate the object, which is *immutable*.
{quote}

Taking CreateTableDesc as an example, I think the only *required* parameters are tableName and cols. Everything else should have a default value that can optionally be overridden using a setter exposed by the Builder.

Another problem with many of these objects (including CreateTableDesc) is that they should be immutable, but currently aren't. This is a consequence of the fact that many of the getters return references to mutable instance variables. For example, CreateTableDesc.getPartCols() returns a reference to the internal list, which means that if I add an element to the list I actually modified the internal state of the CreateTableDesc object. The workaround for this problem is to return a copy of the internal list instead of the list itself.

Also, I saw a lot of raw types on the LHS (e.g. ArrayList instead of List).

                
> Refactor a few classes' constructors with creational patterns
> -------------------------------------------------------------
>
>                 Key: HIVE-3394
>                 URL: https://issues.apache.org/jira/browse/HIVE-3394
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Gang Tim Liu
>            Assignee: Gang Tim Liu
>            Priority: Minor
>         Attachments: HIVE-3394.patch.1
>
>
> It's good to refactor the following classes' constructors with builder/factory pattern. This should be done before HIVE-3072 so that HIVE-3072 can refresh according and extend it to skewed use case:
> 1. MStorageDescriptor.java
> 2. ColumnInfo.java
> 3. ParseContext.java
> 4. CreateTableDesc.java
> 5. ExprNodeColumnDesc.java

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira