fix: reject truncated parallel export paths

CHANGELOG.md

@@ -209,6 +209,33 @@ true until the next version shipped.
   function and a wrong-arity call (`42883`), a null table name (`22004`) and
   `1/0` (`22012`).
 
+- A parallel export refuses a destination too long to hold the names it generates
+  (#863).
+
+  **`pgcolumnar.parallel_export_parquet` checked the destination's length not at
+  all.** Worker paths cross shared memory in `MAXPGPATH` buffers and have a
+  generated name appended, and `snprintf` truncates rather than failing, so a long
+  destination produced short names and the export completed as though nothing had
+  happened.
+
+  Two distinct failures, both silent. A destination of 1000 bytes or more truncated
+  the part names themselves: the run wrote `part-0000.parqu` instead of
+  `part-0000.parquet`, stamped `_SUCCESS` beside it, and reported success for an
+  export `pgcolumnar.read_parquet` does not recognise. A destination of 994 to 999
+  bytes wrote correct part names but truncated the path that
+  `pexport_remove_outputs` composes for its cleanup scan -- `"%s/%s"` from the same
+  directory and the sink's in-flight `part-NNNN.parquet.tmp.<pid>`, which is 30
+  bytes past the directory with a seven-digit pid -- so the scan unlinked a
+  truncated path and left the temporary file behind.
+
+  The destination is now probed against the longest name the file constructs,
+  `"/part-%04d.parquet.tmp.%d"` at `INT_MAX` for both, which is 39 bytes past the
+  directory against the final part name's 24. Anything that does not fit is refused
+  with `54000` before the destination directory is created, because a truncated run
+  publishes a differently named object and still stamps `_SUCCESS` over it.
+
+  The sink itself was never at risk and is unchanged: `columnar_sink.c` builds its
+  temporary name with `psprintf`, which allocates.
 - A shebang and the execute bit go together, and every directory that documents
   a command is swept (#856). Two things were left over from #852.
 

src/columnar_parallel_export.c

@@ -606,6 +606,7 @@ pgcolumnar_parallel_export_parquet(PG_FUNCTION_ARGS)
 				cur;
 	int64		total = 0;
 	int			failed = -1;
+	char		pathProbe[MAXPGPATH];
 
 	if (PG_ARGISNULL(0) || PG_ARGISNULL(1))
 		ereport(ERROR,
@@ -700,6 +701,33 @@ pgcolumnar_parallel_export_parquet(PG_FUNCTION_ARGS)
 				 errmsg("relation \"%s\" is not a columnar table or a partitioned table with columnar partitions",
 						RelationGetRelationName(rel))));
 
+	/*
+	 * Worker paths cross DSM in MAXPGPATH buffers and add a generated part
+	 * name. Refuse a destination that cannot hold the longest possible name;
+	 * truncating it would publish a differently named object and still stamp
+	 * _SUCCESS over an unreadable export.
+	 *
+	 * The longest such name is NOT the final part name. pexport_remove_outputs()
+	 * composes "%s/%s" from this directory and a directory entry into a
+	 * MAXPGPATH buffer, and the entries it acts on include the sink's in-flight
+	 * form part-NNNN.parquet.tmp.<pid>. Measured: "/part-0000.parquet.tmp.PID"
+	 * with a 7-digit pid (this platform's pid_max is 4194304) is dir + 30, while
+	 * the final part name "/part-2147483647.parquet" is only dir + 24, so
+	 * probing the part name alone left a window -- dir lengths 994..999 at
+	 * MAXPGPATH 1024 -- where the destination was accepted and the cleanup scan
+	 * then truncated a path it unlinks. Probe the longest form this file
+	 * constructs instead. The part index and the pid are both ints, so INT_MAX
+	 * bounds each, and the probe is 39 bytes wide against the part name's 24.
+	 */
+	if (snprintf(pathProbe, sizeof(pathProbe), "%s/part-%04d.parquet.tmp.%d",
+				 dir, INT_MAX, INT_MAX) >= (int) sizeof(pathProbe))
+		ereport(ERROR,
+				(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+				 errmsg("parallel export destination is too long"),
+				 errdetail("The destination, the generated part name and the "
+						   "in-flight temporary suffix must fit in %d bytes.",
+						   MAXPGPATH)));
+
 	pexport_prepare_dir(dir);
 
 	/*

test/parallel_export_parquet.sh

@@ -190,6 +190,157 @@ check "retry into the cleaned directory succeeds" \
 check "the retry writes a fresh _SUCCESS marker" \
 	"$([ -f "$DCX/_SUCCESS" ] && echo yes || echo no)" yes
 
+# ---- destination-length guard (#863) ----------------------------------------
+# A destination that fits by itself but not with a generated suffix used to be
+# silently truncated: the export returned success and stamped _SUCCESS beside a
+# clipped part name, which read_parquet ignores.
+#
+# The guard has to probe the LONGEST path this file builds into a fixed
+# MAXPGPATH buffer, and that is NOT the final part name. pexport_remove_outputs()
+# composes "%s/%s" from the directory and a directory entry, and the entries it
+# unlinks include the sink's in-flight form part-NNNN.parquet.tmp.<pid>:
+#
+#     final part name   "/part-2147483647.parquet"           dir + 24
+#     cleanup scan      "/part-0000.parquet.tmp.1234567"     dir + 30
+#
+# 7-digit pids are reachable here (/proc/sys/kernel/pid_max is 4194304), so a
+# guard that probes only the part name accepts dir lengths 994..999 while the
+# cleanup scan's buffer truncates a path it then unlinks. The probe must be the
+# longest constructed form, "/part-<int>.parquet.tmp.<int>", which is 39 bytes
+# at INT_MAX for both numbers -- so the longest acceptable destination is
+# MAXPGPATH - 1 - 39.
+#
+# Both boundaries are asserted: one byte under must still export (this class of
+# fix breaks by becoming over-broad and rejecting legal paths) and one byte over
+# must be refused with ERRCODE_PROGRAM_LIMIT_EXCEEDED, by SQLSTATE -- "it
+# failed" also passes for a typo, a missing table or a dead server.
+
+PEXPORT_TOO_LONG_SQLSTATE=54000		# ERRCODE_PROGRAM_LIMIT_EXCEEDED
+
+# premise 1: the arithmetic is written for MAXPGPATH == 1024.
+PGC_MAXPGPATH="$(sed -n 's/^#define[[:space:]]\{1,\}MAXPGPATH[[:space:]]\{1,\}\([0-9]\{1,\}\).*/\1/p' \
+	"$("$PGC_PG_CONFIG" --includedir-server)/pg_config_manual.h" | head -1)"
+if [ "$PGC_MAXPGPATH" != "1024" ]; then
+	echo "PREMISE FAILED: MAXPGPATH is [$PGC_MAXPGPATH]; these arms are written for 1024" >&2
+	exit 1
+fi
+
+# premise 2: the cleanup scan still builds dir + "/" + entry into a fixed
+# buffer, and the sink still appends ".tmp.<pid>" to the part name. If either
+# moves, the +30 above is stale and these lengths measure nothing.
+pexport_anchor="$(grep -c 'snprintf(fp, sizeof(fp), "%s/%s", dir, de->d_name);' \
+	"$PGC_SRCDIR/src/columnar_parallel_export.c")"
+sink_anchor="$(grep -c 'psprintf("%s.tmp.%d", path, MyProcPid)' \
+	"$PGC_SRCDIR/src/columnar_sink.c")"
+if [ "$pexport_anchor" != "1" ] || [ "$sink_anchor" != "1" ]; then
+	echo "PREMISE FAILED: cleanup-scan anchor [$pexport_anchor] sink .tmp anchor [$sink_anchor], wanted 1 and 1" >&2
+	exit 1
+fi
+
+# Longest destination that still holds the longest generated path, and the
+# shortest destination at which the cleanup scan's "%s/%s" truncates with a
+# 7-digit pid (len("/part-0000.parquet.tmp.1234567") == 30).
+PEXPORT_LEN_OK=$(( PGC_MAXPGPATH - 1 - 39 ))		# 984
+PEXPORT_LEN_OVER=$(( PEXPORT_LEN_OK + 1 ))		# 985
+PEXPORT_LEN_WINDOW=$(( PGC_MAXPGPATH - 30 ))		# 994
+
+# Build a directory path of an EXACT length and create its parents 777, so the
+# server (running as postgres) can create the final component itself. Sets
+# LP_PATH; it does not echo, because an exit inside a command substitution would
+# leave the suite running with an ungated premise.
+LP_PATH=""
+path_of_len() {
+	local want="$1" p="$PGC_WORKDIR/lp" rem
+	while [ $(( want - ${#p} - 1 )) -gt 255 ]; do
+		p="$p/$(printf 'a%.0s' $(seq 1 200))"
+	done
+	rem=$(( want - ${#p} - 1 ))
+	if [ "$rem" -lt 1 ]; then
+		echo "PREMISE FAILED: cannot build a ${want}-byte path under $PGC_WORKDIR" >&2
+		exit 1
+	fi
+	p="$p/$(printf 'a%.0s' $(seq 1 "$rem"))"
+	if [ "${#p}" -ne "$want" ]; then
+		echo "PREMISE FAILED: built a ${#p}-byte path, wanted $want" >&2
+		exit 1
+	fi
+	mkdir -p "$(dirname "$p")" || { echo "PREMISE FAILED: mkdir -p for $want failed" >&2; exit 1; }
+	chmod -R 777 "$PGC_WORKDIR/lp" || { echo "PREMISE FAILED: chmod for $want failed" >&2; exit 1; }
+	if [ ! -d "$(dirname "$p")" ] || [ -e "$p" ]; then
+		echo "PREMISE FAILED: parent missing or target already present for $want" >&2
+		exit 1
+	fi
+	LP_PATH="$p"
+}
+
+# Run one statement ONCE and record its 5-char SQLSTATE, "none" when it
+# succeeded, or HANG on the wall-clock cap. VERBOSITY verbose puts the SQLSTATE
+# and the message on the same ERROR line, so one run yields both, and a caller
+# can assert the message without executing the statement a second time -- a
+# second run of an accepted export hits the require-empty check and returns a
+# DIFFERENT sqlstate (55000), which is exactly how "it failed" lies.
+#
+# Sets globals rather than echoing: a global assigned inside $( ) is assigned in
+# a subshell and never reaches the caller, so SQLSTATE_LAST_OUT would arrive
+# empty and its message check would silently compare nothing.
+SQLSTATE_LAST=""
+SQLSTATE_LAST_OUT=""
+run_sqlstate() {
+	local rc
+	SQLSTATE_LAST=""
+	SQLSTATE_LAST_OUT="$(timeout -s KILL 180 env PATH="$PGC_BINDIR:$PATH" psql \
+		-h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -qtA 2>&1 <<SQLEOF
+\\set VERBOSITY verbose
+$1;
+SQLEOF
+)"
+	rc=$?
+	if [ "$rc" -eq 124 ] || [ "$rc" -eq 137 ]; then SQLSTATE_LAST=HANG; return; fi
+	if grep -q '^ERROR:' <<<"$SQLSTATE_LAST_OUT"; then
+		SQLSTATE_LAST="$(printf '%s\n' "$SQLSTATE_LAST_OUT" |
+			sed -n 's/^ERROR:  \([0-9A-Z]\{5\}\):.*/\1/p' | head -1)"
+	else
+		SQLSTATE_LAST=none
+	fi
+}
+
+# -- accepted boundary: one byte under the limit must still export ------------
+path_of_len "$PEXPORT_LEN_OK"
+PEXPORT_DIR_OK="$LP_PATH"
+check_num "accepted boundary: the destination is exactly MAXPGPATH-40 bytes" \
+	"${#PEXPORT_DIR_OK}" "$PEXPORT_LEN_OK"
+run_sqlstate "SELECT pgcolumnar.parallel_export_parquet('t_col'::regclass, '$PEXPORT_DIR_OK', 2)"
+check "accepted boundary: a ${PEXPORT_LEN_OK}-byte destination still exports" "$SQLSTATE_LAST" none
+check_num "accepted boundary: it wrote both part files" "$(nfiles "$PEXPORT_DIR_OK")" 2
+check "accepted boundary: it wrote a _SUCCESS marker" \
+	"$([ -f "$PEXPORT_DIR_OK/_SUCCESS" ] && echo yes || echo no)" yes
+
+# -- rejected boundary: one byte over the limit --------------------------------
+path_of_len "$PEXPORT_LEN_OVER"
+PEXPORT_DIR_OVER="$LP_PATH"
+check_num "rejected boundary: the destination is exactly MAXPGPATH-39 bytes" \
+	"${#PEXPORT_DIR_OVER}" "$PEXPORT_LEN_OVER"
+run_sqlstate "SELECT pgcolumnar.parallel_export_parquet('t_col'::regclass, '$PEXPORT_DIR_OVER', 2)"
+check "rejected boundary: a ${PEXPORT_LEN_OVER}-byte destination raises 54000" \
+	"$SQLSTATE_LAST" "$PEXPORT_TOO_LONG_SQLSTATE"
+check "rejected boundary: the destination was not created" \
+	"$([ -e "$PEXPORT_DIR_OVER" ] && echo created || echo absent)" absent
+
+# -- the window the part-name-only probe left open ----------------------------
+# dir + 30 overruns the cleanup scan's MAXPGPATH buffer here, while
+# dir + 24 (the final part name) still fits, so the old probe accepted this.
+path_of_len "$PEXPORT_LEN_WINDOW"
+PEXPORT_DIR_WINDOW="$LP_PATH"
+check_num "cleanup-scan window: the destination is exactly MAXPGPATH-30 bytes" \
+	"${#PEXPORT_DIR_WINDOW}" "$PEXPORT_LEN_WINDOW"
+run_sqlstate "SELECT pgcolumnar.parallel_export_parquet('t_col'::regclass, '$PEXPORT_DIR_WINDOW', 2)"
+check "cleanup-scan window: a ${PEXPORT_LEN_WINDOW}-byte destination raises 54000" \
+	"$SQLSTATE_LAST" "$PEXPORT_TOO_LONG_SQLSTATE"
+check "cleanup-scan window: and it is OUR message, not another 54000" \
+	"$(grep -qi 'destination is too long' <<<"$SQLSTATE_LAST_OUT" && echo ours || echo other)" ours
+check "cleanup-scan window: the destination was not created" \
+	"$([ -e "$PEXPORT_DIR_WINDOW" ] && echo created || echo absent)" absent
+
 # ---- error cases ------------------------------------------------------------
 # st_1 was written above, so it is non-empty
 expect_error "reject a non-empty output directory" \