Mailing-List: contact dev-help@madlib.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@madlib.apache.org
From: iyerr3 <git@git.apache.org>
To: dev@madlib.apache.org
Reply-To: dev@madlib.apache.org
References: <git-pr-195-madlib@git.apache.org>
In-Reply-To: <git-pr-195-madlib@git.apache.org>
Subject: [GitHub] madlib pull request #195: Feature: Add grouping support to HITS 
Content-Type: text/plain
Message-Id: <20171120230054.76FC7DFF17@git1-us-west.apache.org>
Date: Mon, 20 Nov 2017 23:00:54 +0000 (UTC)
archived-at: Mon, 20 Nov 2017 23:00:57 -0000

Github user iyerr3 commented on a diff in the pull request:

    https://github.com/apache/madlib/pull/195#discussion_r152136163
  
    --- Diff: src/ports/postgres/modules/utilities/utilities.py_in ---
    @@ -709,16 +709,35 @@ def _check_groups(tbl1, tbl2, grp_list):
         return ' AND '.join([" {tbl1}.{i} = {tbl2}.{i} ".format(**locals())
                              for i in grp_list])
     
    -
    -def _grp_from_table(tbl, grp_list):
    -    """
    -    Helper function for selecting grouping columns of a table
    +def get_filtered_cols_subquery_str(include_from_table, exclude_from_table,
    +                                   filter_cols_list):
    +    """
    +    This function returns a subquery string with columns in the filter_cols_list
    --- End diff --
    
    I'm confused with this description string. My understanding is that the function doesn't really filter the columns - it filters the values for the columns i.e. it returns a subquery string that would filter values in exclude_from_table. If you're interested in filtering columns, then wouldn't querying the catalog and then doing a set difference be the better option? 


---