diff --git a/src/columnar.h b/src/columnar.h index d01c377..c6e6146 100644 --- a/src/columnar.h +++ b/src/columnar.h @@ -517,6 +517,7 @@ typedef struct ColumnarWriteState ColumnarWriteState; extern ColumnarWriteState *ColumnarGetWriteState(Relation rel); extern int ColumnarWriteStateStripeCount(ColumnarWriteState *ws); extern uint64 *ColumnarWriteStateStripeIds(ColumnarWriteState *ws, int *n); +extern uint64 *ColumnarWriteStateProjStripeIds(ColumnarWriteState *ws, int *n); extern uint64 ColumnarWriteRow(ColumnarWriteState *writeState, Relation rel, Datum *values, bool *nulls); extern void ColumnarProjectionFanoutRow(Relation rel, ColumnarWriteState *baseWs, diff --git a/src/columnar_vacuum.c b/src/columnar_vacuum.c index fb4e97b..ee84d7c 100644 --- a/src/columnar_vacuum.c +++ b/src/columnar_vacuum.c @@ -336,11 +336,45 @@ record_online_sorted_extent(Relation rel, uint64 storageId, if (nOurs <= 0) return; /* this rewrite reserved nothing */ - /* our own reservations, ascending */ + /* + * Our own reservations, ascending -- including the ones our projection + * fan-out drew (#345). A projection writes through its own write state but + * reserves from this relation's stripe counter, so its ids interleave with + * ours and would otherwise read as foreign reservations, truncating the run + * at the first projection flush. They are ours: the same transaction drew + * them. Only ids at or above our own first one are taken, so anything drawn + * before this rewrite began is still excluded. + */ ours = (uint64 *) palloc(sizeof(uint64) * nOurs); memcpy(ours, all + stripeMark, sizeof(uint64) * nOurs); qsort(ours, nOurs, sizeof(uint64), uint64_cmp); + { + int nProj = 0; + uint64 *projIds = ColumnarWriteStateProjStripeIds(writeState, &nProj); + + if (nProj > 0) + { + uint64 lo = ours[0]; + uint64 *merged = (uint64 *) palloc(sizeof(uint64) * (nOurs + nProj)); + int m = nOurs; + int j; + + memcpy(merged, ours, sizeof(uint64) * nOurs); + for (j = 0; j < nProj; j++) + { + if (projIds[j] >= lo) + merged[m++] = projIds[j]; + } + pfree(ours); + ours = merged; + nOurs = m; + qsort(ours, nOurs, sizeof(uint64), uint64_cmp); + } + if (projIds != NULL) + pfree(projIds); + } + /* the consecutive run of our own ids, starting at the lowest */ runEnd = ours[0]; for (i = 1; i < nOurs; i++) diff --git a/src/columnar_write_state.c b/src/columnar_write_state.c index b16f6d6..c94e4eb 100644 --- a/src/columnar_write_state.c +++ b/src/columnar_write_state.c @@ -1457,6 +1457,63 @@ typedef struct ColumnarProjWriter MemoryContext rowCtx; /* reset after each stripe flush: row datums */ ColumnarWriteState *innerWs; /* reused stripe encoder for this projection */ } ColumnarProjWriter; +/* + * ColumnarWriteStateProjStripeIds + * The stripe ids this write state's projection fan-out drew (#345). + * + * A projection writes through its own inner write state but reserves from + * the BASE relation's stripe counter, because ColumnarWriteRow is called + * with the base relation (see flush_proj_writer). Its groups are recorded + * under the projection's own storage id, so they never appear in the base + * relation's row group list. + * + * That combination is why the caller needs these separately. To + * record_online_sorted_extent, an id drawn by its own projection fan-out is + * indistinguishable from one taken by another session: both leave a gap in + * the base write state's ids. Treating the former as foreign truncated the + * ordered run at the first projection flush, so a fully reclustered table + * with a projection reported almost all of itself as decayed. + * + * Returns a palloc'd array in the caller's context, or NULL when this write + * state has no projection writers. + */ +uint64 * +ColumnarWriteStateProjStripeIds(ColumnarWriteState *ws, int *n) +{ + ListCell *lc; + uint64 *ids = NULL; + int total = 0; + int k = 0; + + *n = 0; + if (ws->projWriters == NIL) + return NULL; + + foreach(lc, ws->projWriters) + { + ColumnarProjWriter *w = (ColumnarProjWriter *) lfirst(lc); + + if (w->innerWs != NULL) + total += w->innerWs->nReservedStripeIds; + } + if (total == 0) + return NULL; + + ids = (uint64 *) palloc(sizeof(uint64) * total); + foreach(lc, ws->projWriters) + { + ColumnarProjWriter *w = (ColumnarProjWriter *) lfirst(lc); + int i; + + if (w->innerWs == NULL) + continue; + for (i = 0; i < w->innerWs->nReservedStripeIds; i++) + ids[k++] = w->innerWs->reservedStripeIds[i]; + } + *n = k; + return ids; +} + /* * columnar_build_write_state diff --git a/test/recluster_extent.sh b/test/recluster_extent.sh index ee41762..c78c13b 100755 --- a/test/recluster_extent.sh +++ b/test/recluster_extent.sh @@ -144,6 +144,36 @@ check "the id-drawn-below counts still cover every stored row" \ "$(raw "SELECT (sorted_rows + appended_rows = (SELECT sum(rowcount) FROM pgcolumnar.stats('d')))::text FROM pgcolumnar.sort_status('d');")" "true" +# ------------------------------- a projection must not read as foreign (#345) + +# The rewrite's projection fan-out writes through its own write state but draws +# stripe ids from THIS relation's counter, and records its groups under the +# projection's own storage id. So its ids interleave with the rewrite's own and +# are invisible in the base relation's group list -- exactly what a foreign +# reservation looks like. Treating them as foreign truncated the ordered run at +# the first projection flush, so a table that had just been fully reclustered +# reported almost all of itself as decayed. +# +# Both arms are identical except for the projection, so the projection is +# established as the cause rather than assumed. +for arm in noproj withproj; do + raw "DROP TABLE IF EXISTS pr_$arm;" >/dev/null + raw "CREATE TABLE pr_$arm (id int, k int, v text) USING pgcolumnar;" >/dev/null + raw "SELECT pgcolumnar.set_options('pr_$arm', stripe_row_limit => 20000);" >/dev/null + raw "INSERT INTO pr_$arm SELECT g, ((g::bigint * 7919) % 100000)::int, 'v' || g + FROM generate_series(1, 200000) g;" >/dev/null + if [ "$arm" = withproj ]; then + E="$(raw "SELECT pgcolumnar.add_projection('pr_$arm', 'p_$arm', ARRAY['k','id'], ARRAY['k']);")" + case "$E" in *ERROR*) echo " add_projection failed: $E";; esac + fi + raw "SELECT pgcolumnar.recluster('pr_$arm', 'id', 'k');" >/dev/null + S="$(raw "SELECT sorted_rows FROM pgcolumnar.sort_status('pr_$arm');")" + A="$(raw "SELECT appended_rows FROM pgcolumnar.sort_status('pr_$arm');")" + echo " $arm: sorted $S, appended $A" + check "$arm: a full recluster claims every row" "$S" "200000" + check "$arm: a full recluster leaves nothing appended" "$A" "0" +done + # ---------------------------------------------------- the uncontended case # With no concurrent writer the rewrite orders everything, so the run covers the