Clean up code and docs.

Author: abramsh

python/multicorn/__init__.py

@@ -214,16 +214,20 @@ def can_sort(self, sortkeys):
 
     def can_limit(self, limit, offset):
         """
-        Method called from the planner to ask the FDW whether it supports LIMIT pushdown.
-        This method is only called if the rest of the query can be pushed down including the sort and quals. For example,
-        if the query has a GROUP BY clause, this method will not be called.
+        Method called from the planner to ask the FDW whether it supports LIMIT and OFFSET pushdown.
+
+        This method is only called if the entire query can be pushed down. For example, if the query has
+        a GROUP BY clause, this method will not be called. Or, if only part of the sort is pushed down,
+        this method will not be called and limit/offset will not be pushed down.
+        
+        Currently, we do not support pushing down limit/offset if the query includes a WHERE clause (quals).
 
         Args:
-            limit (int or None): The limit to apply to the query.
-            offset (int or None): The offset to apply to the query.
+            limit (int or None): The limit to apply to the query, if any.
+            offset (int or None): The offset to apply to the query, if any.
 
         Return:
-            True if the FDW can support both LIMIT and OFFSET pushdown, Falseotherwise.
+            True if the FDW can support both LIMIT and OFFSET pushdown, False otherwise.
         """
         return False
 
@@ -328,8 +332,8 @@ def execute(self, quals, columns, sortkeys=None, limit=None, offset=None):
                 should be in the sequence.
             sortkeys (list): A list of :class:`SortKey`
                 that the FDW said it can enforce.
-            limit (int): The limit to apply to the query.
-            offset (int): The offset to apply to the query.
+            limit (int or None): The limit to apply to the query, if any.
+            offset (int or None): The offset to apply to the query, if any.
 
         Returns:
             An iterable of python objects which can be converted back to PostgreSQL.

python/multicorn/testfdw.py

@@ -133,10 +133,10 @@ def get_path_keys(self):
         return []
 
     def can_sort(self, sortkeys):
-        # assume sort pushdown ok for all cols, in any order, any collation
+        # assume sort pushdown ok only for first sort key
         if not self.cansort:
             return []
-        return sortkeys
+        return sortkeys[:1]
 
     def can_limit(self, limit, offset):
         return self.canlimit

src/multicorn.c

@@ -375,7 +375,7 @@ multicornGetForeignPaths(PlannerInfo *root,
 	/* Try to find parameterized paths */
 	pathes = findPaths(root, baserel, possiblePaths, planstate->startupCost,
 			planstate, apply_pathkeys, deparsed_pathkeys);
-    
+
 	/* Add a simple default path */
 	pathes = lappend(pathes, create_foreignscan_path(root, baserel,
 		 	NULL,  /* default pathtarget */
@@ -406,6 +406,10 @@ multicornGetForeignPaths(PlannerInfo *root,
 		}
 	}
 
+    /* Determine if the sort is completely pushed down and store the results to be used in the upper paths */
+    /* Regardless, store the deparsed pathkeys to be used in the upper paths */
+    planstate->sort_pushed_down = pathkeys_contained_in(root->sort_pathkeys, apply_pathkeys);
+    
 	/* Add each ForeignPath previously found */
 	foreach(lc, pathes)
 	{
@@ -419,11 +423,13 @@ multicornGetForeignPaths(PlannerInfo *root,
 		{
 			ForeignPath *newpath;
 
-            MulticornPathState *pathstate = (MulticornPathState *)calloc(1, sizeof(MulticornPathState));
+            MulticornPathState *pathstate = (MulticornPathState *)palloc0(sizeof(MulticornPathState));
             pathstate->pathkeys = deparsed_pathkeys;
             pathstate->limit = -1;
             pathstate->offset = -1;
 
+            planstate->pathkeys = deparsed_pathkeys;
+
 			newpath = create_foreignscan_path(root, baserel,
 					NULL,  /* default pathtarget */
 					path->path.rows,
@@ -458,23 +464,21 @@ static void multicornGetForeignUpperPaths(PlannerInfo *root,
                                           RelOptInfo *output_rel, 
                                           void *extra)
 {
-    // elog(WARNING, "Got input_rel private: %p", input_rel->fdw_private);
-    // elog(WARNING, "Got output_rel private: %p", output_rel->fdw_private);
-
-    // If the input_rel has no private, then pushdown wasn't supported for the previous stage
-    // Which means we can't pushdown anything for the the current stage (as least this is true for limit/offset)
+    // If the input_rel has no private, then pushdown wasn't supported for the previous stage. 
+    // Therefore we can't pushdown anything for the the current stage (as least this is true for limit/offset)
     if (!input_rel->fdw_private)
         return;
 
-    // elog(WARNING, "Got stage: %d", stage);
     switch (stage)
 	{
         case UPPERREL_ORDERED:
             add_foreign_ordered_paths(root, input_rel, output_rel, (FinalPathExtraData *)extra);
             break;
+
 		case UPPERREL_FINAL:
             add_foreign_final_paths(root, input_rel, output_rel, (FinalPathExtraData *)extra);
 			break;
+            
 		default:
 			break;
 	}
@@ -484,51 +488,17 @@ static void multicornGetForeignUpperPaths(PlannerInfo *root,
  * add_foreign_ordered_paths
  *		Add foreign paths for performing the sort processing remotely.
  *
- * Given input_rel contains the source-data Paths.  The paths are added to the
- * given final_rel.
- *
  * Note: Since sorts are already taken care of in the base rel, we only check for pushdown here.
  */
  static void
  add_foreign_ordered_paths(PlannerInfo *root, RelOptInfo *input_rel,
                          RelOptInfo *final_rel,
                          FinalPathExtraData *extra)
  {
-    List *applied_pathkeys = NIL;
-    ListCell *lc;
     MulticornPlanState *planstate = input_rel->fdw_private;
-    ForeignPath *cheapest_path = (ForeignPath *)input_rel->cheapest_total_path;
-    if ( planstate )
-    {
-        if (cheapest_path && IsA(cheapest_path, ForeignPath))
-        {
-            MulticornPathState *pathstate = (MulticornPathState *)cheapest_path->fdw_private;
-            if ( pathstate )
-            {
-                planstate->pathkeys = pathstate->pathkeys;
-            }
-        }
-
-        /* Extract the pathkeys from the input_rel */
-        foreach(lc, input_rel->pathlist)
-        {
-            Path *path = (Path *) lfirst(lc);
-            if (IsA(path, ForeignPath))
-            {
-                ForeignPath *fpath = (ForeignPath *) path;
-                if (fpath->path.pathkeys != NIL)
-                {
-                    applied_pathkeys = fpath->path.pathkeys;
-                    break;
-                }
-            }
-        }
-
-        /* We only support limit/offset if the sort is completely pushed down */
-        /* By bailing here, input_rel for the next state will not have planstate, which will cause no more pushdowns */
-        if (!pathkeys_contained_in(root->sort_pathkeys, applied_pathkeys))
-            return;
 
+    if ( planstate && planstate->sort_pushed_down )
+    {
         planstate->input_rel = input_rel;
         final_rel->fdw_private = planstate;
     }
@@ -584,7 +554,7 @@ static void multicornGetForeignUpperPaths(PlannerInfo *root,
     if (parse->limitOffset)
         limitOffset = DatumGetInt32(((Const *)parse->limitOffset)->constvalue);
 
-    /* Get the current input_rel and it's planstate */
+    /* Get the current planstate and its input_rel */
     planstate = input_rel->fdw_private;
     if ( planstate->input_rel )
         input_rel = planstate->input_rel;
@@ -593,11 +563,13 @@ static void multicornGetForeignUpperPaths(PlannerInfo *root,
     if (!canLimit(planstate, limitCount, limitOffset))
         return;
 
-    /* Create foreign final path with the correct number of rows, and include state for limit/offset pushdown */
-    pathstate = (MulticornPathState *)calloc(1, sizeof(MulticornPathState));
+    /* Include pathkeys and limit/offset in pathstate */
+    pathstate = (MulticornPathState *)palloc(sizeof(MulticornPathState));
     pathstate->pathkeys = planstate->pathkeys;
     pathstate->limit = limitCount;
     pathstate->offset = limitOffset;
+
+    /* Create foreign final path with the correct number of rows and cost. */
     final_path = create_foreign_upper_path(root,
                                         input_rel,
                                         root->upper_targets[UPPERREL_FINAL],
@@ -1369,7 +1341,7 @@ serializePlanState(MulticornPlanState * state)
 	List	   *result = NULL;
 
 	result = lappend(result, makeConst(INT4OID,
-					-1, InvalidOid, 4, Int32GetDatum(state->numattrs), false, true));
+					    -1, InvalidOid, 4, Int32GetDatum(state->numattrs), false, true));
 	result = lappend(result, makeConst(INT4OID,
 					-1, InvalidOid, 4, Int32GetDatum(state->foreigntableid), false, true));
 	result = lappend(result, state->target_list);

src/multicorn.h

@@ -93,6 +93,9 @@ typedef struct MulticornPlanState
     int offset;
     int limit;
 
+    /* For tracking if the sort is completely pushed down */
+    bool sort_pushed_down;
+    
     /* Used for tracking the input_rel for upper plans*/
     RelOptInfo *input_rel;
 

test-3.9/expected/multicorn_test_limit.out

@@ -121,6 +121,27 @@ NOTICE:  ['test1', 'test2']
  01-01-2011 | Sun Jan 02 14:30:25 2011
 (1 row)
 
+-- Verify limit and offset are not pushed when sort is partially pushed down
+EXPLAIN SELECT * FROM testmulticorn ORDER BY test1, test2 LIMIT 1 OFFSET 1;
+                                    QUERY PLAN                                    
+----------------------------------------------------------------------------------
+ Limit  (cost=48.08..66.65 rows=1 width=12)
+   ->  Incremental Sort  (cost=29.51..400.90 rows=20 width=12)
+         Sort Key: test1, test2
+         Presorted Key: test1
+         ->  Foreign Scan on testmulticorn  (cost=10.00..400.00 rows=20 width=20)
+(5 rows)
+
+SELECT * FROM testmulticorn ORDER BY test1, test2 LIMIT 1 OFFSET 1;
+NOTICE:  []
+NOTICE:  ['test1', 'test2']
+NOTICE:  requested sort(s): 
+NOTICE:  SortKey(attname='test1', attnum=1, is_reversed=False, nulls_first=False, collate=None)
+   test1    |          test2           
+------------+--------------------------
+ 01-01-2011 | Sun Jan 02 14:30:25 2011
+(1 row)
+
 -- Verify limit and offset are not pushed down with where (which we may eventually support in the future)
 EXPLAIN SELECT * FROM testmulticorn WHERE test1 = '02-03-2011' LIMIT 1 OFFSET 1;
                                  QUERY PLAN                                 

test-3.9/sql/multicorn_test_limit.sql

@@ -55,6 +55,10 @@ SELECT * FROM testmulticorn ORDER BY test1 LIMIT 1 OFFSET 1;
 EXPLAIN SELECT * FROM testmulticorn_nosort ORDER BY test1 LIMIT 1 OFFSET 1;
 SELECT * FROM testmulticorn_nosort ORDER BY test1 LIMIT 1 OFFSET 1;
 
+-- Verify limit and offset are not pushed when sort is partially pushed down
+EXPLAIN SELECT * FROM testmulticorn ORDER BY test1, test2 LIMIT 1 OFFSET 1;
+SELECT * FROM testmulticorn ORDER BY test1, test2 LIMIT 1 OFFSET 1;
+
 -- Verify limit and offset are not pushed down with where (which we may eventually support in the future)
 EXPLAIN SELECT * FROM testmulticorn WHERE test1 = '02-03-2011' LIMIT 1 OFFSET 1;
 SELECT * FROM testmulticorn WHERE test1 = '02-03-2011' LIMIT 1 OFFSET 1;