diff --git a/CHANGELOG.md b/CHANGELOG.md index d1742a1c..f88f82c8 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/columnar_arrow.c b/src/columnar_arrow.c index e239e449..a6b952aa 100644 --- a/src/columnar_arrow.c +++ b/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); } diff --git a/test/arrow_import.sh b/test/arrow_import.sh index 56d2e3d8..2ca09070 100755 --- a/test/arrow_import.sh +++ b/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 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;"