Skip to content

Commit

Permalink
Refactor scanner callback interface
Browse files Browse the repository at this point in the history
This change adds proper result types for the scanner's filter and
tuple handling callbacks. Previously, these callbacks were supposed to
return bool, which was hard to interpret. For instance, for the tuple
handler callback, true meant continue processing the next tuple while
false meant finish the scan. However, this wasn't always clear. Having
proper return types also makes it easier to see from a function's
signature that it is a scanner callback handler, rather than some
other function that can be called directly.
  • Loading branch information
erimatnor committed Nov 8, 2018
1 parent b77b47a commit 28e2e6a
Show file tree
Hide file tree
Showing 13 changed files with 225 additions and 165 deletions.
17 changes: 10 additions & 7 deletions src/bgw/job.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ typedef struct AccumData
size_t alloc_size;
} AccumData;

static bool
static ScanTupleResult
bgw_job_accum_tuple_found(TupleInfo *ti, void *data)
{
AccumData *list_data = data;
Expand All @@ -106,7 +106,7 @@ bgw_job_accum_tuple_found(TupleInfo *ti, void *data)
list_data->list = lappend(list_data->list, job);

MemoryContextSwitchTo(orig);
return true;
return SCAN_CONTINUE;
}


Expand Down Expand Up @@ -166,15 +166,18 @@ bgw_job_scan_job_id(int32 bgw_job_id, tuple_found_func tuple_found, tuple_filter
scankey, 1, tuple_found, tuple_filter, data, mctx, lockmode);
}

static bool
static ScanTupleResult
bgw_job_tuple_found(TupleInfo *ti, void *const data)
{
BgwJob **job_pp = data;

*job_pp = bgw_job_from_tuple(ti->tuple, sizeof(BgwJob), ti->mctx);

/* return true since used with scan one */
return true;
/*
* Return SCAN_CONTINUE because we check for multiple tuples as an error
* condition.
*/
return SCAN_CONTINUE;
}

BgwJob *
Expand Down Expand Up @@ -227,7 +230,7 @@ ts_bgw_job_insert_relation(PG_FUNCTION_ARGS)
PG_RETURN_NULL();
}

static bool
static ScanTupleResult
bgw_job_tuple_delete(TupleInfo *ti, void *data)
{
CatalogSecurityContext sec_ctx;
Expand All @@ -239,7 +242,7 @@ bgw_job_tuple_delete(TupleInfo *ti, void *data)
catalog_delete(ti->scanrel, ti->tuple);
catalog_restore_user(&sec_ctx);

return true;
return SCAN_CONTINUE;
}

static bool
Expand Down
29 changes: 15 additions & 14 deletions src/bgw/job_stat.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,18 @@ bgw_job_stat_next_start_was_set(FormData_bgw_job_stat *fd)
}


static bool
static ScanTupleResult
bgw_job_stat_tuple_found(TupleInfo *ti, void *const data)
{
BgwJobStat **job_stat_pp = data;

*job_stat_pp = bgw_job_stat_from_tuple(ti->tuple, ti->mctx);

/* return true because used with scan_one */
return true;
/*
* Return SCAN_CONTINUE because we check for multiple tuples as an error
* condition.
*/
return SCAN_CONTINUE;
}

static bool
Expand Down Expand Up @@ -89,11 +92,12 @@ bgw_job_stat_find(int32 bgw_job_id)
return job_stat;
}

static bool
static ScanTupleResult
bgw_job_stat_tuple_delete(TupleInfo *ti, void *const data)
{
catalog_delete(ti->scanrel, ti->tuple);
return true;

return SCAN_CONTINUE;
}

void
Expand All @@ -109,7 +113,7 @@ bgw_job_stat_delete(int32 bgw_job_id)
* instance can write once a crash happened in any job. Therefore our only choice is to deduce
* a crash from the lack of a write (the marked end write in this case).
*/
static bool
static ScanTupleResult
bgw_job_stat_tuple_mark_start(TupleInfo *ti, void *const data)
{
HeapTuple tuple = heap_copytuple(ti->tuple);
Expand Down Expand Up @@ -142,8 +146,7 @@ bgw_job_stat_tuple_mark_start(TupleInfo *ti, void *const data)
catalog_update(ti->scanrel, tuple);
heap_freetuple(tuple);

/* scans with catalog_update must return false */
return false;
return SCAN_DONE;
}


Expand Down Expand Up @@ -204,7 +207,7 @@ calculate_next_start_on_crash(int consecutive_crashes, BgwJob *job)
return failure_calc;
}

static bool
static ScanTupleResult
bgw_job_stat_tuple_mark_end(TupleInfo *ti, void *const data)
{
JobResultCtx *result_ctx = data;
Expand Down Expand Up @@ -246,11 +249,10 @@ bgw_job_stat_tuple_mark_end(TupleInfo *ti, void *const data)
catalog_update(ti->scanrel, tuple);
heap_freetuple(tuple);

/* scans with catalog_update must return false */
return false;
return SCAN_DONE;
}

static bool
static ScanTupleResult
bgw_job_stat_tuple_set_next_start(TupleInfo *ti, void *const data)
{
TimestampTz *next_start = data;
Expand All @@ -262,8 +264,7 @@ bgw_job_stat_tuple_set_next_start(TupleInfo *ti, void *const data)
catalog_update(ti->scanrel, tuple);
heap_freetuple(tuple);

/* scans with catalog_update must return false */
return false;
return SCAN_DONE;
}

static bool
Expand Down

0 comments on commit 28e2e6a

Please sign in to comment.