fix: report the nanosecond narrowing instead of doing it silently

CHANGELOG.md

@@ -18,6 +18,43 @@ true until the next version shipped.
 
 ### Added
 
+- A nanosecond Arrow import says how many values lost precision, and still
+  imports every row.
+
+  **`pgcolumnar.import_arrow` narrows nanoseconds to microseconds and always
+  did.** PostgreSQL timestamps and times are int64 microseconds; Arrow
+  parameterises the unit in the type. Of Arrow's four units, second,
+  millisecond and microsecond all widen or match exactly, so only nanosecond can
+  lose anything, and only for a value that is not already on a microsecond
+  boundary.
+
+  The narrowing was silent, and that is what changed. An import now ends with
+
+      NOTICE:  columnar.import_arrow: 12 values lost sub-microsecond precision
+      DETAIL:  PostgreSQL timestamps and times hold microseconds; this Arrow file
+               declares nanoseconds.
+      HINT:    Import the raw nanoseconds into a bigint column if the extra digits
+               are significant.
+
+  **Counted per value, not per type.** A pandas `datetime64[ns]` column built
+  from second- or millisecond-resolution data is nanosecond-TYPED and entirely
+  lossless to convert; it reports nothing. Only values that actually carried
+  sub-microsecond digits are counted. The remainder is not extra work:
+  `arrow_floordiv` already computes it to decide the rounding direction.
+
+  The counter is reached from the time arm as well as the timestamp arm, and is
+  threaded through the list and struct recursion, so a nested `timestamp[]` and a
+  `time64[ns]` column are counted the same way.
+
+  **Nothing is refused.** `NOTICE`, not `WARNING` or `ERROR`: a bulk load must
+  not fail on the last row of a large file for a conversion the caller may have
+  intended. Out-of-range values are still refused with `22008`, unchanged.
+
+  Truncation does more than reduce precision -- it can make rows that were
+  distinct in the file compare EQUAL here, so a `UNIQUE` violation or a lost
+  `ORDER BY` tie-break now has its cause stated at the point it was introduced
+  rather than surfacing later as a mystery.
+
 - The two visibility-map clears that no test held are now held (#877).
 
   **Three paths retire a row group and give its live rows new row numbers**, and

src/columnar_arrow.c

@@ -1505,8 +1505,22 @@ arrow_floordiv(int64 num, int64 den)
 	return q;
 }
 
+/*
+ * Scale an Arrow temporal value onto PostgreSQL's microseconds.
+ *
+ * Of the four Arrow units, second, millisecond and microsecond all widen or
+ * match, so only nanosecond can lose anything -- and only for a value that is
+ * not already on a microsecond boundary. A pandas datetime64[ns] column built
+ * from second- or millisecond-resolution data is nanosecond-TYPED and entirely
+ * lossless to convert, which is why the loss is counted per VALUE and not
+ * refused per TYPE.
+ *
+ * nsTrunc, when not NULL, is incremented once per value that actually lost
+ * digits. The remainder is not extra work: arrow_floordiv already computes it
+ * to decide the rounding direction.
+ */
 static bool
-arrow_scale_to_usecs(int unit, int64 v, int64 *out)
+arrow_scale_to_usecs(int unit, int64 v, int64 *out, int64 *nsTrunc)
 {
 	switch (unit)
 	{
@@ -1515,6 +1529,8 @@ arrow_scale_to_usecs(int unit, int64 v, int64 *out)
 		case ARROW_TU_MILLI:
 			return !pg_mul_s64_overflow(v, INT64CONST(1000), out);
 		case ARROW_TU_NANO:
+			if (nsTrunc != NULL && v % INT64CONST(1000) != 0)
+				(*nsTrunc)++;
 			*out = arrow_floordiv(v, INT64CONST(1000));
 			return true;
 		case ARROW_TU_MICRO:
@@ -1654,7 +1670,7 @@ imp_apply_field(ImpNode *n, const uint8 *meta, uint32 metaLen, uint32 field)
 
 static Datum
 imp_scalar_at(ImpNode *n, const uint8 *body, const int64 *bufOff,
-			  const int64 *bufLen, int64 i)
+			  const int64 *bufLen, int64 i, int64 *nsTrunc)
 {
 	if (n->kind == A_BOOL)
 	{
@@ -1804,7 +1820,7 @@ imp_scalar_at(ImpNode *n, const uint8 *body, const int64 *bufOff,
 					 * midnight for a malformed input.
 					 */
 					if (raw < INT64CONST(0) ||
-						!arrow_scale_to_usecs(n->srcUnit, raw, &us) ||
+						!arrow_scale_to_usecs(n->srcUnit, raw, &us, nsTrunc) ||
 						us > USECS_PER_DAY)
 						ereport(ERROR,
 								(errcode(ERRCODE_DATETIME_FIELD_OVERFLOW),
@@ -1819,7 +1835,7 @@ imp_scalar_at(ImpNode *n, const uint8 *body, const int64 *bufOff,
 					int64		t;
 
 					memcpy(&raw, vp, 8);
-					if (!arrow_scale_to_usecs(n->srcUnit, raw, &us) ||
+					if (!arrow_scale_to_usecs(n->srcUnit, raw, &us, nsTrunc) ||
 						pg_sub_s64_overflow(us, PG_TO_UNIX_USECS, &t) ||
 						!IS_VALID_TIMESTAMP((Timestamp) t))
 						ereport(ERROR,
@@ -1859,7 +1875,7 @@ imp_scalar_at(ImpNode *n, const uint8 *body, const int64 *bufOff,
 /* reconstruct a node's value at index i (recursively for list/struct) */
 static Datum
 imp_value_at(ImpNode *n, const uint8 *body, const int64 *bufOff,
-			 const int64 *bufLen, int64 i, bool *isnull)
+			 const int64 *bufLen, int64 i, bool *isnull, int64 *nsTrunc)
 {
 	*isnull = imp_is_null(body, bufOff[n->validBuf], bufLen[n->validBuf], i);
 	if (*isnull)
@@ -1885,7 +1901,7 @@ imp_value_at(ImpNode *n, const uint8 *body, const int64 *bufOff,
 		enulls = (nelem > 0) ? palloc(sizeof(bool) * nelem) : NULL;
 		for (k = 0; k < nelem; k++)
 			elems[k] = imp_value_at(&n->children[0], body, bufOff, bufLen,
-									start + k, &enulls[k]);
+									start + k, &enulls[k], nsTrunc);
 		dims[0] = nelem;
 		arr = construct_md_array(elems, enulls, 1, dims, lbs, n->elemtype,
 								 n->elemlen, n->elembyval, n->elemalign);
@@ -1907,12 +1923,13 @@ imp_value_at(ImpNode *n, const uint8 *body, const int64 *bufOff,
 				fn[a] = true;
 				continue;
 			}
-			fv[a] = imp_value_at(&n->children[ci++], body, bufOff, bufLen, i, &fn[a]);
+			fv[a] = imp_value_at(&n->children[ci++], body, bufOff, bufLen, i,
+								 &fn[a], nsTrunc);
 		}
 		tup = heap_form_tuple(n->structDesc, fv, fn);
 		return HeapTupleGetDatum(tup);
 	}
-	return imp_scalar_at(n, body, bufOff, bufLen, i);
+	return imp_scalar_at(n, body, bufOff, bufLen, i, nsTrunc);
 }
 
 /*
@@ -2036,6 +2053,7 @@ pgcolumnar_import_arrow(PG_FUNCTION_ARGS)
 	CommandId	cid;
 	MemoryContext rowCtx;
 	int64		total = 0;
+	int64		nsTrunc = 0;	/* values that lost sub-microsecond digits */
 	bool		sawSchema = false;
 	int			i;
 
@@ -2267,7 +2285,8 @@ pgcolumnar_import_arrow(PG_FUNCTION_ARGS)
 						bool		isnull;
 
 						slot->tts_values[i] = imp_value_at(&tops[i], body, bufOff,
-														   bufLen, r, &isnull);
+														   bufLen, r, &isnull,
+														   &nsTrunc);
 						slot->tts_isnull[i] = isnull;
 					}
 					ExecStoreVirtualTuple(slot);
@@ -2319,5 +2338,33 @@ pgcolumnar_import_arrow(PG_FUNCTION_ARGS)
 	 * INSERT.
 	 */
 	table_close(rel, NoLock);
+
+	/*
+	 * Report the narrowing rather than refuse it. An import that changed the
+	 * data must not be indistinguishable from one that did not: truncation to
+	 * the microsecond can make rows that were distinct in the file EQUAL here,
+	 * so a later UNIQUE violation or a lost ORDER BY tie-break has its cause
+	 * stated where it was introduced instead of surfacing as a mystery.
+	 *
+	 * NOTICE, not WARNING or ERROR: nothing is malformed and nothing is
+	 * refused, and a bulk load must not die on the last row of a large file for
+	 * a conversion the caller may well have intended. Counted per VALUE, so a
+	 * nanosecond-TYPED file whose values all sit on microsecond boundaries --
+	 * which is what pandas produces from second- or millisecond-resolution data
+	 * -- says nothing at all.
+	 *
+	 * No denominator. nsTrunc counts VALUES and total counts ROWS, so "N of M"
+	 * divides one population by another: three nanosecond columns over four rows
+	 * printed "12 of 4 values", both numbers true and the sentence joining them
+	 * false. A correct denominator would need a second counter for temporal
+	 * values converted, and the count alone is what the reader acts on.
+	 */
+	if (nsTrunc > 0)
+		ereport(NOTICE,
+				(errmsg("columnar.import_arrow: " INT64_FORMAT
+						" values lost sub-microsecond precision", nsTrunc),
+				 errdetail("PostgreSQL timestamps and times hold microseconds; this Arrow file declares nanoseconds."),
+				 errhint("Import the raw nanoseconds into a bigint column if the extra digits are significant.")));
+
 	PG_RETURN_INT64(total);
 }

test/arrow_import.sh

@@ -126,6 +126,121 @@ check "non-finite imported as null" "$nulls" "1"
 keep="$(q "SELECT count(*) FROM ri_nf2 WHERE id=2 AND dt='2020-01-01' AND num=1.25;")"
 check "finite row preserved" "$keep" "1"
 
+# --- 3b. documented lossy mapping: nanosecond -> microsecond, and it is REPORTED
+#
+# PostgreSQL timestamps and times are int64 MICROSECONDS; Arrow parameterises the
+# unit in the type. Of the four Arrow units, second, millisecond and microsecond
+# all widen or match exactly, so only nanosecond can lose anything -- and only for
+# values that are not already on a microsecond boundary. A pandas datetime64[ns]
+# column built from second- or millisecond-resolution data is nanosecond-TYPED and
+# entirely lossless to convert.
+#
+# The contract is: never refuse over this. Narrow it, keep every row, and say how
+# many values actually lost digits. Silence would make an import that changed the
+# data indistinguishable from one that did not -- and truncation does more than
+# reduce precision, it can make distinct rows EQUAL, which is what a later UNIQUE
+# violation would be reporting without explaining.
+#
+# The two controls are the point of the section: an ns-typed file whose values are
+# all on a microsecond boundary must report NOTHING, and a microsecond file must
+# report nothing, or the counter is measuring the unit rather than the loss.
+if [ "$have_pyarrow" = 1 ]; then
+	echo "-- nanosecond narrowing is reported, never refused"
+	psql_c() { env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" \
+		-U postgres -d "$PGC_DB" -At -c "$1" 2>&1; }
+
+	NSLOSSY="$PGC_WORKDIR/ns_lossy.arrows"
+	NSEXACT="$PGC_WORKDIR/ns_exact.arrows"
+	USPLAIN="$PGC_WORKDIR/us_plain.arrows"
+	NSDEEP="$PGC_WORKDIR/ns_deep.arrows"
+	python3 - "$NSLOSSY" "$NSEXACT" "$USPLAIN" "$NSDEEP" <<'PY'
+import sys, pyarrow as pa, pyarrow.ipc as ipc
+S2000 = 946684800
+def w(path, a, b):
+    t = pa.table({'ts': a, 'ts2': b})
+    with ipc.new_stream(pa.OSFile(path, 'wb'), t.schema) as o:
+        o.write_table(t)
+# TWO temporal columns over four rows, so the number of VALUES that lose digits
+# (8) differs from the number of ROWS (4). With one column the two coincide and
+# an arm cannot tell which one the message is reporting.
+def ns(off):
+    return pa.array([(S2000 + i) * 10**9 + off for i in range(4)], pa.timestamp('ns'))
+# every value carries 123ns past a microsecond boundary: all EIGHT truncate
+w(sys.argv[1], ns(123), ns(123))
+# nanosecond-TYPED but microsecond-exact: nothing is lost, nothing to report
+w(sys.argv[2], ns(0), ns(0))
+# a different unit entirely: the counter must not fire on it
+us = lambda: pa.array([(S2000 + i) * 10**6 for i in range(4)], pa.timestamp('us'))
+w(sys.argv[3], us(), us())
+# The counter is reached from TWO call sites -- the time arm and the timestamp
+# arm -- and is threaded through the list and struct recursion. A file with only
+# a top-level timestamp column leaves the other three paths unmeasured: passing
+# NULL at the time site, or dropping the counter from the nested recursion,
+# would not move a single arm above. This file covers them.
+#   time64[ns]                  4 values, all 123ns past a boundary
+#   list<timestamp[ns]> x2      8 values, likewise
+#                              12 total
+NOON = 12 * 3600
+tm = pa.array([(NOON + i) * 10**9 + 123 for i in range(4)], pa.time64('ns'))
+lst = pa.array([[(S2000 + i) * 10**9 + 123, (S2000 + i + 1) * 10**9 + 123]
+                for i in range(4)], pa.list_(pa.timestamp('ns')))
+deep = pa.table({'tm': tm, 'lst': lst})
+with ipc.new_stream(pa.OSFile(sys.argv[4], 'wb'), deep.schema) as o:
+    o.write_table(deep)
+PY
+
+	psql_run "CREATE TABLE ri_nsl (ts timestamp, ts2 timestamp) USING pgcolumnar;"
+	psql_run "CREATE TABLE ri_nse (ts timestamp, ts2 timestamp) USING pgcolumnar;"
+	psql_run "CREATE TABLE ri_usp (ts timestamp, ts2 timestamp) USING pgcolumnar;"
+
+	nsl_out="$(psql_c "SELECT pgcolumnar.import_arrow('ri_nsl', '$NSLOSSY');")"
+	nse_out="$(psql_c "SELECT pgcolumnar.import_arrow('ri_nse', '$NSEXACT');")"
+	usp_out="$(psql_c "SELECT pgcolumnar.import_arrow('ri_usp', '$USPLAIN');")"
+
+	# THE CONTRACT: not one row refused, on any of the three.
+	check "a lossy nanosecond file imports every row" \
+		"$(q "SELECT count(*) FROM ri_nsl;")" "4"
+	check "so does a microsecond-exact nanosecond file" \
+		"$(q "SELECT count(*) FROM ri_nse;")" "4"
+	check "and a microsecond file" \
+		"$(q "SELECT count(*) FROM ri_usp;")" "4"
+
+	# THE ARM: the loss is counted and reported, once, with the number -- and the
+	# number is the count of VALUES (8), not of rows (4). The fixture has two
+	# temporal columns precisely so those two differ: with one column they
+	# coincide, and an arm matching "4" is satisfied by either, which is how an
+	# earlier revision of this suite passed a build whose counter reported the
+	# wrong population entirely.
+	check "the narrowing reports the number of VALUES that lost digits, not rows" \
+		"$(printf '%s' "$nsl_out" | grep -c 'columnar.import_arrow: 8 values lost sub-microsecond precision')" "1"
+	check "and does not report the row count instead" \
+		"$(printf '%s' "$nsl_out" | grep -c 'import_arrow: 4 values lost')" "0"
+
+	# The other three paths that reach the counter. Without this arm, passing
+	# NULL for nsTrunc at the time call site, or dropping it from the list or
+	# struct recursion, leaves every arm above green.
+	psql_run "CREATE TABLE ri_nsd (tm time, lst timestamp[]) USING pgcolumnar;"
+	nsd_out="$(psql_c "SELECT pgcolumnar.import_arrow('ri_nsd', '$NSDEEP');")"
+	check "a time64 column and a nested list are counted too, not just a top-level timestamp" \
+		"$(printf '%s' "$nsd_out" | grep -c 'columnar.import_arrow: 12 values lost sub-microsecond precision')" "1"
+	check "and that file imports every row as well" \
+		"$(q "SELECT count(*) FROM ri_nsd;")" "4"
+
+	# THE CONTROLS: no report when nothing was lost.
+	check "control: an ns file on microsecond boundaries reports nothing" \
+		"$(printf '%s' "$nse_out" | grep -ci 'sub-microsecond')" "0"
+	check "control: a microsecond file reports nothing" \
+		"$(printf '%s' "$usp_out" | grep -ci 'sub-microsecond')" "0"
+
+	# The conversion itself is unchanged: floor to the microsecond, not rounded
+	# and not refused. 123ns past the boundary lands ON the boundary.
+	check "the narrowed values are floored to the microsecond" \
+		"$(q "SELECT string_agg(ts::text, ',' ORDER BY ts) FROM ri_nsl;")" \
+		"2000-01-01 00:00:00,2000-01-01 00:00:01,2000-01-01 00:00:02,2000-01-01 00:00:03"
+else
+	echo "-- pyarrow not available; skipping nanosecond narrowing checks"
+fi
+
 # --- 4. error cases ---------------------------------------------------------
 echo "-- argument validation"
 psql_run "CREATE TABLE ri_heap (a bigint, b float8, c text) USING heap;"