fix: reject out-of-range Arrow temporal values

CHANGELOG.md

@@ -35,6 +35,45 @@ true until the next version shipped.
 
 ### Fixed
 
+- `pgcolumnar.import_arrow` rejects an out-of-range Arrow temporal value instead
+  of storing an invalid `Datum` (#862).
+
+  Arrow carries `date32`, `time64`, and `timestamp` as bare integers over the
+  carrier's full range. PostgreSQL's `date`, `time`, and `timestamp` are
+  narrower. The importer converted without checking, so a foreign file could put
+  a value into a columnar table that no PostgreSQL operator can read back.
+  Import now raises `22008` (`ERRCODE_DATETIME_VALUE_OUT_OF_RANGE`) on four
+  conditions:
+
+  - a `date32` outside `IS_VALID_DATE`,
+  - a `time64` outside `[0, USECS_PER_DAY)`,
+  - a `timestamp` whose epoch shift would underflow `int64`,
+  - a `timestamp` outside `IS_VALID_TIMESTAMP` after that shift.
+
+  **The suite asserts the SQLSTATE, not merely that it failed.** `22008` is the
+  claim. "The statement errored" is also true of a wrong path, a missing table,
+  or a malformed IPC stream. It is true of this file's own dictionary-encoding
+  rejection too. So each arm was measured red with its own guard reverted. Each
+  was green with the other three guards intact. The four runs used four
+  separately fingerprinted builds. Each red reports an absent SQLSTATE, not a
+  different one.
+
+  Two things the arms measured along the way. First, `IS_VALID_TIMESTAMP`'s
+  **upper** bound is unreachable through this path. `ts >= END_TIMESTAMP` needs
+  an Arrow value of at least 9224318016000000000. That exceeds `INT64_MAX`, so
+  any such value underflows the subtraction first and the underflow guard takes
+  it. Only the lower bound is testable, and the arm uses it.
+
+  Second, the underflow guard is not merely hygiene against undefined behaviour.
+  With it removed, the `INT64_MIN` row imports successfully and reads back as
+  `294247-01-10 04:00:54.775808`. That is measured, not derived. Whatever the
+  wrapped subtraction produced, `IS_VALID_TIMESTAMP` accepted it. The guard is
+  the only thing between a hostile file and a stored garbage date.
+
+  When pyarrow is absent the four arms cannot run, so they report `UNRUN`
+  (`MISSING_DEPENDENCY`) and the suite exits `INCOMPLETE`. They previously would
+  have vanished inside a suite still reporting `PASSED`.
+
 - 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.
 

docs/sql-reference.md

@@ -475,6 +475,11 @@ Inserts the rows of an Arrow IPC stream file at `path` into the existing table
 `rel`. The column types of the table define the types that the function accepts. Returns the number of
 rows inserted.
 
+Arrow carries `date32`, `time64`, and `timestamp` as bare integers. Their range
+is wider than what PostgreSQL's `date`, `time`, and `timestamp` hold. A value
+the target type cannot represent raises `22008` (`datetime_field_overflow`).
+The import fails and stores nothing.
+
 ### pgcolumnar.import_parquet(rel regclass, path text) returns bigint
 
 Inserts the rows of a Parquet file at `path` into the existing table `rel`. The

src/columnar_arrow.c

@@ -1541,24 +1541,44 @@ imp_scalar_at(ImpNode *n, const uint8 *body, const int64 *bufOff,
 			case A_DATE32:
 				{
 					int32		v;
+					int64		d;
 
 					memcpy(&v, vp, 4);
-					return DateADTGetDatum((DateADT) (v - PG_TO_UNIX_DAYS));
+					d = (int64) v - PG_TO_UNIX_DAYS;
+					if (!IS_VALID_DATE(d))
+						ereport(ERROR,
+								(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+								 errmsg("columnar.import_arrow: date value is out of range")));
+					return DateADTGetDatum((DateADT) d);
 				}
 			case A_TIME64:
 				{
 					int64		v;
 
 					memcpy(&v, vp, 8);
+					if (v < 0 || v >= USECS_PER_DAY)
+						ereport(ERROR,
+								(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+								 errmsg("columnar.import_arrow: time value is out of range")));
 					return TimeADTGetDatum(v);
 				}
 			case A_TIMESTAMP:
 			case A_TIMESTAMPTZ:
 				{
 					int64		v;
+					Timestamp	ts;
 
 					memcpy(&v, vp, 8);
-					return TimestampGetDatum((Timestamp) (v - PG_TO_UNIX_USECS));
+					if (v < PG_INT64_MIN + PG_TO_UNIX_USECS)
+						ereport(ERROR,
+								(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+								 errmsg("columnar.import_arrow: timestamp value is out of range")));
+					ts = (Timestamp) (v - PG_TO_UNIX_USECS);
+					if (!IS_VALID_TIMESTAMP(ts))
+						ereport(ERROR,
+								(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+								 errmsg("columnar.import_arrow: timestamp value is out of range")));
+					return TimestampGetDatum(ts);
 				}
 			case A_UUID:
 				{

test/arrow_import.sh

@@ -45,6 +45,13 @@ fifo_release() { exec 9<>"$1" 2>/dev/null; exec 9>&- 2>/dev/null; }
 have_pyarrow=1
 python3 -c 'import pyarrow' 2>/dev/null || have_pyarrow=0
 
+# One name per arm, used by the arm and by its unrunnable counterpart below, so
+# the two cannot drift apart.
+OOB_DATE_ARM="reject out-of-range Arrow date as 22008"
+OOB_TIME_ARM="reject out-of-range Arrow time as 22008"
+OOB_TS_LOW_ARM="reject underflowing Arrow timestamp as 22008"
+OOB_TS_RANGE_ARM="reject Arrow timestamp before the PostgreSQL minimum as 22008"
+
 expect_error() {
 	local label="$1" sql="$2"
 	if psql_run "$sql" >/dev/null 2>&1; then
@@ -144,6 +151,95 @@ with ipc.new_stream(pa.OSFile(sys.argv[1], 'wb'), t.schema) as w:
 PY
 	psql_run "CREATE TABLE ri_d (c text) USING pgcolumnar;"
 	expect_error "reject dictionary-encoded file" "SELECT pgcolumnar.import_arrow('ri_d', '$DICTF');"
+
+	# Arrow temporal values span the integer carrier's full range, while
+	# PostgreSQL's date, time, and timestamp types have narrower valid ranges.
+	# Import must reject those values instead of overflowing or storing an
+	# invalid internal Datum.
+	#
+	# "it failed" is not the claim. The claim is that the value is refused as OUT
+	# OF RANGE, which is ERRCODE_DATETIME_VALUE_OUT_OF_RANGE = 22008. An arm that
+	# only asserts a non-zero exit also passes for a wrong path, a missing table,
+	# a malformed IPC stream, a different pyarrow, or this file's own
+	# dictionary-encoding rejection -- none of which is the property under test.
+	# These fixtures are hand-built buffers (pa.Array.from_buffers), so that
+	# difference is not theoretical. Assert the SQLSTATE.
+	#
+	# Four values for four guards, one value per guard, so a revert of any single
+	# guard reddens exactly one arm:
+	#   date_oob       INT32_MIN days   -> !IS_VALID_DATE
+	#   time_oob       -1 usec          -> v < 0 || v >= USECS_PER_DAY
+	#   timestamp_oob  INT64_MIN usec   -> v < PG_INT64_MIN + PG_TO_UNIX_USECS
+	#   timestamp_pre_min  -2^60 usec   -> !IS_VALID_TIMESTAMP, on its LOWER bound.
+	#
+	# IS_VALID_TIMESTAMP's upper bound is not reachable through this path and no
+	# fixture can cover it: ts >= END_TIMESTAMP needs v >= 9224318016000000000,
+	# which exceeds INT64_MAX, so any such Arrow value underflows the subtraction
+	# first and is caught by the guard above. -2^60 is chosen because it is below
+	# MIN_TIMESTAMP after the epoch shift yet nowhere near the underflow
+	# threshold (v < -9222425352054775808), so it pins the range guard alone.
+	# 2^62 does NOT work here and was measured doing so: it is a perfectly valid
+	# PostgreSQL timestamp (about year 148000) and imports successfully.
+	#
+	# The fixture is a premise, not a printout: if the writer fails or pyarrow
+	# stops accepting these buffers, the arms below would assert a SQLSTATE
+	# against a file that does not exist and report a plausible wrong answer. Gate
+	# on it and stop.
+	if ! python3 - "$PGC_WORKDIR" <<'PY'
+import os, struct, sys, pyarrow as pa, pyarrow.ipc as ipc
+out = sys.argv[1]
+cases = {
+    'date_oob.arrows': (pa.date32(), struct.pack('<i', -(2**31))),
+    'time_oob.arrows': (pa.time64('us'), struct.pack('<q', -1)),
+    'timestamp_oob.arrows': (pa.timestamp('us'), struct.pack('<q', -(2**63))),
+    'timestamp_pre_min.arrows': (pa.timestamp('us'), struct.pack('<q', -(2**60))),
+}
+for name, (typ, raw) in cases.items():
+    arr = pa.Array.from_buffers(typ, 1, [None, pa.py_buffer(raw)])
+    table = pa.table({'v': arr})
+    with ipc.new_stream(pa.OSFile(os.path.join(out, name), 'wb'), table.schema) as w:
+        w.write_table(table)
+PY
+	then
+		echo "arrow_import.sh: could not build the out-of-range temporal fixtures" >&2
+		exit 1
+	fi
+	for oob_fixture in date_oob time_oob timestamp_oob timestamp_pre_min; do
+		if [ ! -s "$PGC_WORKDIR/$oob_fixture.arrows" ]; then
+			echo "arrow_import.sh: fixture $oob_fixture.arrows is missing or empty" >&2
+			exit 1
+		fi
+	done
+
+	psql_run "CREATE TABLE ri_date_oob (v date) USING pgcolumnar;
+	          CREATE TABLE ri_time_oob (v time) USING pgcolumnar;
+	          CREATE TABLE ri_timestamp_oob (v timestamp) USING pgcolumnar;
+	          CREATE TABLE ri_timestamp_pre_min (v timestamp) USING pgcolumnar;"
+	# sqlstate_or_hang prints the bare 5-char SQLSTATE, or HANG, or nothing at all
+	# when the statement SUCCEEDED. check_text refuses an empty side outright, so
+	# "the guard is gone and the value imported" cannot read as anything but red.
+	check_text "$OOB_DATE_ARM" \
+		"$(sqlstate_or_hang "SELECT pgcolumnar.import_arrow('ri_date_oob', '$PGC_WORKDIR/date_oob.arrows')")" \
+		"22008"
+	check_text "$OOB_TIME_ARM" \
+		"$(sqlstate_or_hang "SELECT pgcolumnar.import_arrow('ri_time_oob', '$PGC_WORKDIR/time_oob.arrows')")" \
+		"22008"
+	check_text "$OOB_TS_LOW_ARM" \
+		"$(sqlstate_or_hang "SELECT pgcolumnar.import_arrow('ri_timestamp_oob', '$PGC_WORKDIR/timestamp_oob.arrows')")" \
+		"22008"
+	check_text "$OOB_TS_RANGE_ARM" \
+		"$(sqlstate_or_hang "SELECT pgcolumnar.import_arrow('ri_timestamp_pre_min', '$PGC_WORKDIR/timestamp_pre_min.arrows')")" \
+		"22008"
+else
+	# These four arms live inside the pyarrow block. Without pyarrow they do not
+	# run, and an arm that silently does not run while the suite still reports
+	# PASSED is the failure mode this file has had before. Name them as the third
+	# state instead; the suite then exits INCOMPLETE rather than claiming a pass
+	# for a question nobody asked. (The pre-existing dictionary-encoding arm above
+	# has the same exposure; it is left alone here because it is not this change.)
+	for oob_arm in "$OOB_DATE_ARM" "$OOB_TIME_ARM" "$OOB_TS_LOW_ARM" "$OOB_TS_RANGE_ARM"; do
+		check_unrunnable "$oob_arm" MISSING_DEPENDENCY "pyarrow is not importable"
+	done
 fi
 
 IXFILE="$PGC_WORKDIR/ix_roundtrip.arrows"