From d13385ac0b5e5f050880876c72f986cd98538fa4 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Tue, 1 Sep 2026 19:13:17 +0000 Subject: [PATCH 1/2] fix: read the temporal unit and carrier width the Arrow file declares import_arrow built its decode plan from the target column type alone and never opened the Arrow Field table, so the file's carrier width and temporal unit were never read. One root cause, three symptoms (#864, #865). Measured on unpatched main, all silent: date64 2000-01-01 -> 4908285-05-04 timestamp(s) 2000-01-01 -> 1970-01-01 00:15:46.6848 timestamp(ms) 2000-01-01 -> 1970-01-11 22:58:04.8 timestamp(ns) 2000-01-01 -> 31969-04-01 time64(ns) 12:00:00 -> 12000:00:00 time32(s|ms) 12:00:00 -> import fails, "value buffer too small" Only the microsecond timestamp and time units and date32 were read correctly, and those are exactly what export_arrow writes, so a round trip through our own exporter never showed it. The Schema walk now reads Date.unit, Time.unit, Time.bitWidth and Timestamp.unit, and scales to PostgreSQL's units. An absent field means its FlatBuffers default, and two of those defaults are not zero: measured against pyarrow, date64 and time32[ms] carry no unit field at all. Reading an absent field as 0 is what produced the date64 result above. Scaling is overflow-checked with pg_mul_s64_overflow / pg_sub_s64_overflow and range-checked, refusing with 22008 rather than wrapping; C signed overflow is undefined behaviour, not a wraparound. Nanoseconds narrow to microseconds, which keeps the instant where reading ns as us is wrong by 1000x. Every narrowing floors, so an instant before the epoch reports the day and microsecond it falls in. Nested fields are walked too, so a timestamp[] or a composite with a temporal member is read by its own unit. n->width is both the decode stride and the divisor in imp_check_bounds, while each non-temporal decode arm reads a size fixed by its kind. A width taken from a tag the target does not share separates the two, so the walk stamps a node only when the file's tag matches the node's kind. Without that gate a Date-tagged field set width 4 under an A_INT64 node that still reads 8 bytes, passing the bounds check and reading off the end of the body. That check also closes a third case, present since Arrow import shipped: a temporal file the target column cannot hold was read anyway from the low four bytes of an eight-byte carrier. Measured on main, a time64 file into a date column stored 687342-02-27 and a timestamp file into one stored 2722128-09-17. Both are now refused with 42804. A non-temporal tag is left alone, so an int64 file into a timestamp column still imports. test/arrow_corpus.py had no temporal columns at all, so fuzz_arrow could not reach any of this; it now carries ten temporal seeds. Tests: test/arrow_import.sh 49 checks, red before green, each new guarantee proved by removal. The raw-sign guard on time is reported honestly as redundant under flooring and kept as defence in depth. Gate: arrow_import, arrow_export, arrow_nested_import, fuzz_arrow, import_deferred, import_exclusion, import_export_privilege, export_sink, entry_point_privilege, row_triggers, rls_direct_storage, server_file_privilege, local_open_race_free, native_parquet_units, parquet_import, parquet_export -- 16/16 on pg18a and pg19a; the four Arrow suites green on pg15a..pg19a. --- CHANGELOG.md | 43 ++++++ docs/sql-reference.md | 15 +++ src/columnar_arrow.c | 303 ++++++++++++++++++++++++++++++++++++++++-- test/arrow_corpus.py | 40 ++++++ test/arrow_import.sh | 211 +++++++++++++++++++++++++++++ 5 files changed, 600 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9d2e43c..333c91c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,49 @@ true until the next version shipped. ### Fixed +- Arrow import reads the temporal unit and carrier width the file declares, + rather than assuming the ones our own exporter writes (#864, #865). + + **A file could say what it meant and be read as something else.** The importer + built its decode plan from the target column type alone and never opened the + Arrow `Field` table, so the unit went unread. A `date64` column holding + 2000-01-01 was decoded as a day count and stored as `4908285-05-04`; a + `timestamp` in seconds became `1970-01-01 00:15:46.6848`, in milliseconds + `1970-01-11 22:58:04.8`, and in nanoseconds `31969-04-01`. A `time64` in + nanoseconds holding noon stored `12000:00:00`, a legal PostgreSQL time. None + raised an error. `time32` could not be imported at all, failing with "value + buffer too small for the row count". + + Only microsecond timestamps and times, and `date32` dates, were read + correctly, and those are exactly what `export_arrow` writes -- so a round trip + through our own exporter never showed the defect. + + The import now reads `Date.unit`, `Time.unit`, `Time.bitWidth` and + `Timestamp.unit` from the file and scales to PostgreSQL's units, treating an + absent field as its FlatBuffers default. That last part matters: a writer omits + any field equal to its default, and two of these defaults are not zero, so + pyarrow emits `date64` and `time32[ms]` with no unit field at all. Reading an + absent field as zero is what produced the `date64` result above. + + Scaling a coarse unit up can leave PostgreSQL's range, so each conversion is + overflow-checked and refused with `22008` rather than wrapped. Nanoseconds are + narrowed to microseconds, which PostgreSQL cannot store beyond: narrowing keeps + the instant, where reading nanoseconds as microseconds is wrong by a factor of + 1000. Every narrowing floors, so an instant before the epoch reports the day and + the microsecond it falls in rather than the one after. + + The unit is applied to nested fields too, so a `timestamp[]` or a composite + with a temporal member is read by its own declared unit. + + Reading the file's declared type also closed a third case, found while fixing + these two and present since Arrow import shipped: a temporal file whose type + the target column cannot hold was read anyway, taking the low four bytes of an + eight-byte carrier. A `time64` file imported into a `date` column stored + `687342-02-27`, and a `timestamp` file into the same column stored + `2722128-09-17`. Both are now refused with `42804`, naming the Arrow type and + the column type. A file whose type is not temporal is unaffected, so importing + a plain `int64` file into a `timestamp` column still works. + - 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. diff --git a/docs/sql-reference.md b/docs/sql-reference.md index 37ba96dd..431c662e 100644 --- a/docs/sql-reference.md +++ b/docs/sql-reference.md @@ -475,6 +475,21 @@ 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. +Temporal columns are read in the unit the file declares, not the unit +`export_arrow` writes. A `date` column accepts `date32` and `date64`; a `time` +column accepts `time32` in seconds or milliseconds and `time64` in microseconds +or nanoseconds; a `timestamp` column accepts any of the four `Timestamp` units. +Values are converted to PostgreSQL's own units, and a value that cannot be +represented is refused with `22008` rather than stored wrong. + +Nanoseconds are narrowed to microseconds, the finest resolution PostgreSQL +stores. Narrowing floors, so an instant before 1970 reports the microsecond it +falls in. Sub-microsecond precision is lost; nothing else is. + +A temporal Arrow type the target column cannot hold is refused with `42804` +rather than read: a `time64` file does not import into a `date` column. A file +whose type is not temporal is unaffected. + ### pgcolumnar.import_parquet(rel regclass, path text) returns bigint Inserts the rows of a Parquet file at `path` into the existing table `rel`. The diff --git a/src/columnar_arrow.c b/src/columnar_arrow.c index cfa01631..e239e449 100644 --- a/src/columnar_arrow.c +++ b/src/columnar_arrow.c @@ -36,6 +36,7 @@ #include "access/tableam.h" #include "access/xact.h" #include "catalog/pg_type.h" +#include "common/int.h" #include "executor/tuptable.h" #include "lib/stringinfo.h" #include "catalog/pg_authid_d.h" @@ -74,6 +75,25 @@ PG_FUNCTION_INFO_V1(pgcolumnar_import_arrow); #define ARROW_TYPE_Date 8 #define ARROW_TYPE_Time 9 #define ARROW_TYPE_Timestamp 10 + +/* + * Arrow DateUnit and TimeUnit (Schema.fbs). + * + * A FlatBuffers writer omits any field equal to its schema default, and two of + * these defaults are not zero: Date.unit defaults to MILLISECOND, and Time.unit + * to MILLISECOND with bitWidth 32. pyarrow therefore writes date64 and time32[ms] + * with no unit field at all. Reading an absent field as 0 is what made a date64 + * file decode as a day count (#864). + */ +#define ARROW_DU_DAY 0 +#define ARROW_DU_MILLI 1 /* Date.unit default */ +#define ARROW_TU_SECOND 0 /* Timestamp.unit default */ +#define ARROW_TU_MILLI 1 /* Time.unit default */ +#define ARROW_TU_MICRO 2 +#define ARROW_TU_NANO 3 + +/* milliseconds in a day, for the date64 carrier */ +#define ARROW_MSECS_PER_DAY INT64CONST(86400000) #define ARROW_TYPE_List 12 #define ARROW_TYPE_Struct 13 #define ARROW_TYPE_FixedSizeBinary 15 @@ -1207,6 +1227,11 @@ fbr_u32(const uint8 *b, uint32 len, uint32 pos) memcpy(&v, b + pos, 4); return v; } +static int16 +fbr_i16(const uint8 *b, uint32 len, uint32 pos) +{ + return (int16) fbr_u16(b, len, pos); +} static int32 fbr_i32(const uint8 *b, uint32 len, uint32 pos) { @@ -1320,8 +1345,9 @@ typedef struct ImpNode { ArrowKind kind; Oid typid; - int width; - int scale; + int width; /* carrier bytes IN THE FILE, not in PostgreSQL */ + int scale; /* decimal scale; not a temporal unit */ + int srcUnit; /* Arrow DateUnit/TimeUnit, -1 if the file said nothing */ int32 atttypmod; bool needsInput; FmgrInfo inFinfo; @@ -1351,6 +1377,7 @@ imp_build_node(ImpNode *n, Oid typid, int32 typmod, bool *ok) n->typid = typid; n->atttypmod = typmod; n->validBuf = n->offBuf = n->dataBuf = -1; + n->srcUnit = -1; elemtype = get_element_type(typid); if (OidIsValid(elemtype)) @@ -1448,6 +1475,183 @@ imp_assign_buffers(ImpNode *n, int *bufcur) } /* decode a scalar leaf value at index i (caller checked non-null) */ +/* + * Convert a stored TIME/TIMESTAMP value to the microseconds PostgreSQL stores, + * per the unit the FILE declares. Returns false if the conversion overflows. + * + * This mirrors pq_scale_to_usecs in columnar_parquet_reader.c, and deliberately + * makes the same two calls: NANOS is divided rather than refused, because + * PostgreSQL has no nanosecond timestamp and truncating yields the right instant + * whereas reading nanoseconds as microseconds is wrong by a factor of 1000; and + * a unit the file did not declare is read as microseconds, which is what our own + * exporter writes. Arrow adds a SECOND unit that Parquet does not have. + * + * If one reader's policy changes the other must change with it. + */ +/* + * Floor division. C division truncates toward zero, which for a negative value + * names the unit AFTER the one it falls in: -1500 nanoseconds is 1.5us before + * the epoch, and -1500/1000 == -1 places it 1us before instead. Every narrowing + * here floors, so the date arm and the timestamp arm cannot disagree about which + * day or microsecond an instant belongs to. + */ +static int64 +arrow_floordiv(int64 num, int64 den) +{ + int64 q = num / den; + + if (num % den != 0 && (num < 0) != (den < 0)) + q--; + return q; +} + +static bool +arrow_scale_to_usecs(int unit, int64 v, int64 *out) +{ + switch (unit) + { + case ARROW_TU_SECOND: + return !pg_mul_s64_overflow(v, INT64CONST(1000000), out); + case ARROW_TU_MILLI: + return !pg_mul_s64_overflow(v, INT64CONST(1000), out); + case ARROW_TU_NANO: + *out = arrow_floordiv(v, INT64CONST(1000)); + return true; + case ARROW_TU_MICRO: + default: + *out = v; + return true; + } +} + +/* + * Apply what the file's Field table declares onto an import node. + * + * imp_build_node knows only what PostgreSQL wants. The carrier width and the + * temporal unit live in the file and the target type does not imply either: + * Arrow gives date32 and date64 the same type tag (8) and time32 and time64 the + * same tag (9), so the tag alone never settles the width. + * + * An absent field means its FlatBuffers default, which is not zero for Date or + * Time -- see the ARROW_DU_/ARROW_TU_ comment above. + * + * Recursion is driven by the node tree, which comes from the target type, so a + * file cannot drive it deeper than the column's own nesting. + */ +static void +imp_temporal_mismatch(const char *arrowtype, Oid typid) +{ + ereport(ERROR, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("Arrow field is %s, which columnar.import_arrow cannot read into type %s", + arrowtype, format_type_be(typid)))); +} + +static void +imp_apply_field(ImpNode *n, const uint8 *meta, uint32 metaLen, uint32 field) +{ + uint32 pos; + uint32 tt; + uint8 typetag; + + if (field == 0) + return; + + pos = pgc_fb_field(meta, metaLen, field, 2); /* type_type (u8) */ + typetag = pos ? fbr_u8(meta, metaLen, pos) : 0; + pos = pgc_fb_field(meta, metaLen, field, 3); /* type (offset) */ + tt = pos ? pgc_fb_indirect(meta, metaLen, pos) : 0; + + if (tt != 0) + { + /* + * Stamp a node ONLY when the file's tag matches the kind the target type + * gave it. n->width is both the stride and the divisor in + * imp_check_bounds, while every non-temporal decode arm memcpy's a size + * fixed by its kind. Taking a width from a tag the target does not share + * severs those two, and a Date-tagged field would set width 4 under an + * A_INT64 node that still reads 8 bytes -- passing the bounds check and + * reading off the end of the body. A temporal tag under a target that + * cannot hold it is refused by name. Leaving it alone kept a pre-existing + * silent corruption: measured on unpatched main, a time64 file into a date + * column stored 687342-02-27 and a timestamp file into one stored + * 2722128-09-17, both from the low 4 bytes of an 8-byte carrier. A + * NON-temporal tag is still left alone, so an int64 file into a timestamp + * column keeps working exactly as it does today. + */ + switch (typetag) + { + case ARROW_TYPE_Date: + if (n->kind != A_DATE32) + imp_temporal_mismatch("a date", n->typid); + pos = pgc_fb_field(meta, metaLen, tt, 0); /* unit (i16) */ + n->srcUnit = pos ? fbr_i16(meta, metaLen, pos) : ARROW_DU_MILLI; + if (n->srcUnit != ARROW_DU_DAY && n->srcUnit != ARROW_DU_MILLI) + IMPORT_CORRUPT("unknown Arrow DateUnit"); + n->width = (n->srcUnit == ARROW_DU_DAY) ? 4 : 8; + break; + case ARROW_TYPE_Time: + { + int32 bits; + + if (n->kind != A_TIME64) + imp_temporal_mismatch("a time", n->typid); + pos = pgc_fb_field(meta, metaLen, tt, 0); /* unit (i16) */ + n->srcUnit = pos ? fbr_i16(meta, metaLen, pos) : ARROW_TU_MILLI; + pos = pgc_fb_field(meta, metaLen, tt, 1); /* bitWidth (i32) */ + bits = pos ? fbr_i32(meta, metaLen, pos) : 32; + if (bits != 32 && bits != 64) + IMPORT_CORRUPT("Arrow Time bitWidth is neither 32 nor 64"); + + /* + * The spec pairs the two: Time32 is s or ms, Time64 is us or + * ns. Refuse a mismatched pair rather than scale a carrier by + * a unit that cannot belong to it. + */ + if (bits == 32 && n->srcUnit != ARROW_TU_SECOND && + n->srcUnit != ARROW_TU_MILLI) + IMPORT_CORRUPT("Arrow Time32 unit is neither second nor millisecond"); + if (bits == 64 && n->srcUnit != ARROW_TU_MICRO && + n->srcUnit != ARROW_TU_NANO) + IMPORT_CORRUPT("Arrow Time64 unit is neither microsecond nor nanosecond"); + n->width = bits / 8; + break; + } + case ARROW_TYPE_Timestamp: + if (n->kind != A_TIMESTAMP && n->kind != A_TIMESTAMPTZ) + imp_temporal_mismatch("a timestamp", n->typid); + pos = pgc_fb_field(meta, metaLen, tt, 0); /* unit (i16) */ + n->srcUnit = pos ? fbr_i16(meta, metaLen, pos) : ARROW_TU_SECOND; + if (n->srcUnit < ARROW_TU_SECOND || n->srcUnit > ARROW_TU_NANO) + IMPORT_CORRUPT("unknown Arrow TimeUnit"); + n->width = 8; + break; + default: + break; + } + } + + /* a list or struct carries its element types as children (Field slot 5) */ + if (n->nchildren > 0) + { + uint32 vec; + uint32 cnt; + int i; + + pos = pgc_fb_field(meta, metaLen, field, 5); + if (pos == 0) + return; + vec = pgc_fb_indirect(meta, metaLen, pos); + cnt = fbr_u32(meta, metaLen, vec); + if (cnt != (uint32) n->nchildren) + IMPORT_CORRUPT("Arrow field child count does not match the target type"); + for (i = 0; i < n->nchildren; i++) + imp_apply_field(&n->children[i], meta, metaLen, + pgc_fb_indirect(meta, metaLen, + vec + 4 + (uint32) i * 4)); + } +} + static Datum imp_scalar_at(ImpNode *n, const uint8 *body, const int64 *bufOff, const int64 *bufLen, int64 i) @@ -1540,25 +1744,90 @@ imp_scalar_at(ImpNode *n, const uint8 *body, const int64 *bufOff, } case A_DATE32: { - int32 v; + int64 days; - memcpy(&v, vp, 4); - return DateADTGetDatum((DateADT) (v - PG_TO_UNIX_DAYS)); + if (n->width == 8) + { + int64 ms; + + /* date64: milliseconds from the Unix epoch */ + memcpy(&ms, vp, 8); + days = ms / ARROW_MSECS_PER_DAY; + + /* + * C division truncates toward zero, which names the day + * AFTER the one an instant before the epoch falls on. + * Floor instead, so every instant reports the date it is in. + */ + if (ms % ARROW_MSECS_PER_DAY != 0 && ms < 0) + days--; + } + else + { + int32 v; + + memcpy(&v, vp, 4); + days = v; + } + days -= PG_TO_UNIX_DAYS; + if (!IS_VALID_DATE(days)) + ereport(ERROR, + (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW), + errmsg("columnar: Arrow date value out of range for type date"))); + return DateADTGetDatum((DateADT) days); } case A_TIME64: { - int64 v; + int64 raw; + int64 us; - memcpy(&v, vp, 8); - return TimeADTGetDatum(v); + if (n->width == 4) + { + int32 v; + + memcpy(&v, vp, 4); + raw = v; + } + else + memcpy(&raw, vp, 8); + + /* + * TimeADT is microseconds since midnight; nothing else is a + * time. + * + * The sign is tested on the STORED value. Flooring already + * carries a negative count to a negative microsecond count, so + * this guard is redundant today and reddens no test on its own + * -- measured. It is kept because it does not depend on the + * narrowing rule: truncation toward zero would take -500ns to + * exactly 0, and a check on the scaled result would then store + * midnight for a malformed input. + */ + if (raw < INT64CONST(0) || + !arrow_scale_to_usecs(n->srcUnit, raw, &us) || + us > USECS_PER_DAY) + ereport(ERROR, + (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW), + errmsg("columnar: Arrow time value out of range for type time"))); + return TimeADTGetDatum((TimeADT) us); } case A_TIMESTAMP: case A_TIMESTAMPTZ: { - int64 v; - - memcpy(&v, vp, 8); - return TimestampGetDatum((Timestamp) (v - PG_TO_UNIX_USECS)); + int64 raw; + int64 us; + int64 t; + + memcpy(&raw, vp, 8); + if (!arrow_scale_to_usecs(n->srcUnit, raw, &us) || + pg_sub_s64_overflow(us, PG_TO_UNIX_USECS, &t) || + !IS_VALID_TIMESTAMP((Timestamp) t)) + ereport(ERROR, + (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW), + errmsg("columnar: Arrow timestamp value out of range for type %s", + n->kind == A_TIMESTAMP ? "timestamp" + : "timestamp with time zone"))); + return TimestampGetDatum((Timestamp) t); } case A_UUID: { @@ -1918,6 +2187,16 @@ pgcolumnar_import_arrow(PG_FUNCTION_ARGS) (errcode(ERRCODE_DATATYPE_MISMATCH), errmsg("Arrow file has %u columns, target table has %d", nfields, ncols))); + + /* + * Take the carrier width and temporal unit from the file. Before this + * the Schema was only counted, so every temporal column was decoded as + * whatever the target type happened to be (#864, #865). + */ + for (i = 0; i < ncols; i++) + imp_apply_field(&tops[i], meta, metaLen, + pgc_fb_indirect(meta, metaLen, + fieldsVec + 4 + (uint32) i * 4)); sawSchema = true; } else if (headerType == ARROW_MSG_RecordBatch) diff --git a/test/arrow_corpus.py b/test/arrow_corpus.py index 6d6136a0..abbc1507 100755 --- a/test/arrow_corpus.py +++ b/test/arrow_corpus.py @@ -40,6 +40,19 @@ "bool": "bool", "string": "text", "binary": "bytea", + # Temporal carriers. Both Date widths and both Time widths map to one + # PostgreSQL type each, so the file's declared unit is the only thing that + # says how wide a value is -- which is what the mutations here get to attack. + "date32[day]": "date", + "date64[ms]": "date", + "time32[s]": "time", + "time32[ms]": "time", + "time64[us]": "time", + "time64[ns]": "time", + "timestamp[s]": "timestamp", + "timestamp[ms]": "timestamp", + "timestamp[us]": "timestamp", + "timestamp[ns]": "timestamp", } @@ -101,6 +114,33 @@ def main(): w(outdir, "string", [batch([("c0", pa.array(["v%d" % i for i in range(n)]))])]) w(outdir, "binary", [batch([("c0", pa.array([b"\x00\x01%d" % i for i in range(n)], pa.binary()))])]) + # --- temporal carriers and units. The decoder takes its stride and its + # scale factor from the file's own Date/Time/Timestamp tables, so a mutation + # in those bytes reaches arithmetic no other seed exercises. Without these + # the unit-aware decode path was unreachable from this corpus entirely. + S2000 = 946684800 + NOON = 12 * 3600 + w(outdir, "date32", [batch([("c0", pa.array( + [S2000 // 86400 + i for i in range(n)], pa.date32()))])]) + w(outdir, "date64", [batch([("c0", pa.array( + [(S2000 + i * 86400) * 1000 for i in range(n)], pa.date64()))])]) + w(outdir, "time32_s", [batch([("c0", pa.array( + [(NOON + i) % 86400 for i in range(n)], pa.time32("s")))])]) + w(outdir, "time32_ms", [batch([("c0", pa.array( + [((NOON + i) % 86400) * 1000 for i in range(n)], pa.time32("ms")))])]) + w(outdir, "time64_us", [batch([("c0", pa.array( + [((NOON + i) % 86400) * 10**6 for i in range(n)], pa.time64("us")))])]) + w(outdir, "time64_ns", [batch([("c0", pa.array( + [((NOON + i) % 86400) * 10**9 for i in range(n)], pa.time64("ns")))])]) + w(outdir, "ts_s", [batch([("c0", pa.array( + [S2000 + i for i in range(n)], pa.timestamp("s")))])]) + w(outdir, "ts_ms", [batch([("c0", pa.array( + [(S2000 + i) * 1000 for i in range(n)], pa.timestamp("ms")))])]) + w(outdir, "ts_us", [batch([("c0", pa.array( + [(S2000 + i) * 10**6 for i in range(n)], pa.timestamp("us")))])]) + w(outdir, "ts_ns", [batch([("c0", pa.array( + [(S2000 + i) * 10**9 for i in range(n)], pa.timestamp("ns")))])]) + # --- null shapes: the validity bitmap is a separate buffer and a separate # decode path from the values. w(outdir, "nulls_some", [batch([("c0", pa.array( diff --git a/test/arrow_import.sh b/test/arrow_import.sh index c39a6a6e..56d2e3d8 100755 --- a/test/arrow_import.sh +++ b/test/arrow_import.sh @@ -144,6 +144,217 @@ 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');" + + # ---- temporal carriers and units (#864, #865) ----------------------------- + # + # Arrow gives Date two carriers (DAY on 4 bytes, MILLISECOND on 8) and gives + # Timestamp and Time four units each (s, ms, us, ns). The reader decoded every + # one of them as though it were the single shape our own exporter writes, so a + # VALID file in any other shape imported as a different, valid-looking value: + # + # date64 2000-01-01 -> 4908285-05-04 + # timestamp(s) 2000-01-01 -> 1970-01-01 00:15:46.6848 + # timestamp(ms) 2000-01-01 -> 1970-01-11 22:58:04.8 + # timestamp(ns) 2000-01-01 -> 31969-04-01 + # time64(ns) 12:00:00 -> 12000:00:00 + # + # These assert the VALUE, not that an error was raised. A wrong value cannot + # satisfy them for an unrelated reason the way a deny arm can: a missing + # fixture or a renamed table gives an empty result, not the right timestamp. + # Every file below holds a value that is valid in its own unit, so nothing here + # is testing overflow handling -- that is what the out-of-range arms are for. + python3 - "$PGC_WORKDIR" <<'PY' +import os, struct, sys, pyarrow as pa, pyarrow.ipc as ipc +out = sys.argv[1] +S2000 = 946684800 # 2000-01-01T00:00:00Z in seconds +NOON = 12 * 3600 # 12:00:00 in seconds +cases = { + 'tu_date32': (pa.date32(), S2000 // 86400), + 'tu_date64': (pa.date64(), S2000 * 1000), + 'tu_ts_s': (pa.timestamp('s'), S2000), + 'tu_ts_ms': (pa.timestamp('ms'), S2000 * 1000), + 'tu_ts_us': (pa.timestamp('us'), S2000 * 1000000), + 'tu_ts_ns': (pa.timestamp('ns'), S2000 * 1000000000), + 'tu_t64_us': (pa.time64('us'), NOON * 1000000), + 'tu_t64_ns': (pa.time64('ns'), NOON * 1000000000), + 'tu_t32_s': (pa.time32('s'), NOON), + 'tu_t32_ms': (pa.time32('ms'), NOON * 1000), + # guard inputs: each is valid for its carrier and wrong for PostgreSQL + 'tu_ts_s_ovf': (pa.timestamp('s'), 2**63 - 1), # x1e6 overflows int64 + 'tu_ts_ms_far': (pa.timestamp('ms'), -300000000000000), # far past, below MIN_TIMESTAMP + 'tu_d64_frac': (pa.date64(), 86400001), # not a whole day + 'tu_d64_neg': (pa.date64(), -1), # 1969-12-31T23:59:59.999 + 'tu_t64_ns_ovf': (pa.time64('ns'), 25 * 3600 * 10**9),# past midnight + 'tu_ts_ns_trunc':(pa.timestamp('ns'), S2000 * 10**9 + 1500), # 1.5us past the epoch + 'tu_t64_ns_neg': (pa.time64('ns'), -500), # narrows to 0us: not midnight + 'tu_ts_ns_neg': (pa.timestamp('ns'), -1500), # 1.5us BEFORE the epoch + # a temporal carrier whose tag no other temporal target can hold + 'tu_t64_plain': (pa.time64('us'), NOON * 10**6), +} +for name, (typ, v) in cases.items(): + raw = struct.pack(' the stored value, or the empty string + psql_run "DROP TABLE IF EXISTS tu_t; CREATE TABLE tu_t (v $2) USING pgcolumnar;" \ + >/dev/null 2>&1 + psql_run "SELECT pgcolumnar.import_arrow('tu_t', '$PGC_WORKDIR/$1.arrows');" \ + >/dev/null 2>&1 || { echo "IMPORT-FAILED"; return; } + q "SELECT v::text FROM tu_t LIMIT 1;" + } + + # PREMISE. The unit our own exporter writes must still round-trip, or every + # check below could pass on a reader that rejects everything. + check "premise: the exporter's own timestamp unit still imports correctly" \ + "$(tu_value tu_ts_us timestamp)" "2000-01-01 00:00:00" + check "premise: and its own time unit does too" \ + "$(tu_value tu_t64_us time)" "12:00:00" + check "premise: and a date32 carrier, which shares Arrow's tag 8 with date64" \ + "$(tu_value tu_date32 date)" "2000-01-01" + + check "a date64 carrier decodes to the date it holds (#864)" \ + "$(tu_value tu_date64 date)" "2000-01-01" + + check "a timestamp in seconds decodes to the instant it holds (#865)" \ + "$(tu_value tu_ts_s timestamp)" "2000-01-01 00:00:00" + check "a timestamp in milliseconds decodes to the instant it holds (#865)" \ + "$(tu_value tu_ts_ms timestamp)" "2000-01-01 00:00:00" + check "a timestamp in nanoseconds decodes to the instant it holds (#865)" \ + "$(tu_value tu_ts_ns timestamp)" "2000-01-01 00:00:00" + + check "a time64 in nanoseconds decodes to the time it holds (#865)" \ + "$(tu_value tu_t64_ns time)" "12:00:00" + check "a time32 in seconds decodes to the time it holds (#865)" \ + "$(tu_value tu_t32_s time)" "12:00:00" + check "a time32 in milliseconds decodes to the time it holds (#865)" \ + "$(tu_value tu_t32_ms time)" "12:00:00" + + # ---- the guards the unit fix introduced ------------------------------- + # + # Scaling a coarse unit up can leave the value outside what PostgreSQL can + # store, and in C a signed overflow is undefined behaviour rather than a + # wraparound, so each of these has to be refused before the multiply lands. + # 22008 is datetime_field_overflow. + tu_import_state() { # tu_import_state FIXTURE PGTYPE -> SQLSTATE, 00000 on success + psql_run "DROP TABLE IF EXISTS tu_t; CREATE TABLE tu_t (v $2) USING pgcolumnar;" \ + >/dev/null 2>&1 + local st + st="$(sqlstate_or_hang "SELECT pgcolumnar.import_arrow('tu_t', '$PGC_WORKDIR/$1.arrows')")" + [ -z "$st" ] && st=00000 + printf '%s\n' "$st" + } + + check "premise: a well-formed import reports 00000, so the probe reads success" \ + "$(tu_import_state tu_ts_us timestamp)" "00000" + + check "a second count that overflows on scaling is refused, not wrapped" \ + "$(tu_import_state tu_ts_s_ovf timestamp)" "22008" + # Far PAST, not far future. The two bounds are not symmetric here: exceeding + # END_TIMESTAMP needs 9224318016000000000 microseconds, which is larger than + # INT64_MAX, so for every unit the overflow guard fires before the range check + # can. Only the lower bound is reachable, so only it can be asserted. + check "a timestamp before PostgreSQL's range is refused" \ + "$(tu_import_state tu_ts_ms_far timestamp)" "22008" + check "a time past midnight is refused" \ + "$(tu_import_state tu_t64_ns_ovf time)" "22008" + + # date64 is milliseconds, so an instant need not land on midnight. The date + # it falls IN is the floor, and C division truncates toward zero: without a + # floor correction a pre-epoch instant reports the day after the one it is in. + check "a date64 instant mid-day reports the day it falls in" \ + "$(tu_value tu_d64_frac date)" "1970-01-02" + check "a date64 instant before the epoch floors rather than truncating" \ + "$(tu_value tu_d64_neg date)" "1969-12-31" + + # PostgreSQL has no nanosecond timestamp, so sub-microsecond precision cannot + # survive; truncating keeps the instant, where reading ns as us is 1000x wrong. + check "a nanosecond timestamp truncates to microseconds" \ + "$(tu_value tu_ts_ns_trunc timestamp)" "2000-01-01 00:00:00.000001" + + # Narrowing floors rather than truncating toward zero, so an instant before + # the epoch reports the microsecond and the day it is IN. Truncation would + # move both of these forward, and would disagree with the date64 arm above. + check "a nanosecond timestamp before the epoch floors rather than truncating" \ + "$(tu_value tu_ts_ns_neg timestamp)" "1969-12-31 23:59:59.999998" + check "a negative nanosecond time is refused, not narrowed into midnight" \ + "$(tu_import_state tu_t64_ns_neg time)" "22008" + + # ---- the file's declared type must match the target's ------------------- + # + # n->width is both the decode stride and the divisor in the row-count bounds + # check, while each non-temporal decode arm reads a size fixed by its kind. + # Taking a width from a tag the target does not share separates the two: a + # Date-tagged field under a bigint node gave stride 4 to an arm reading 8, + # which passed the bounds check and read past the body. Every arm below is + # refused on unpatched main with XX001; these pin that it stays refused. + for tu_target in bigint uuid time "numeric(20,4)" timestamp; do + check "a date32 file is refused for a $tu_target column, not read past its buffer" \ + "$(tu_import_state tu_d32_3 "$tu_target")" "42804" + done + + # The other direction, and the reason the refusal is by name rather than a + # bounds error. These three were ACCEPTED on unpatched main, storing the low + # 4 bytes of an 8-byte carrier as a plausible value: 687342-02-27, + # 2722128-09-17, and 1970-01-11 22:58:04.8 respectively. + check "a time64 file is refused for a date column, not stored as year 687342" \ + "$(tu_import_state tu_t64_plain date)" "42804" + check "a timestamp file is refused for a date column" \ + "$(tu_import_state tu_ts_us date)" "42804" + check "a date64 file is refused for a timestamp column" \ + "$(tu_import_state tu_date64 timestamp)" "42804" + + # A NON-temporal tag is still left alone: an int64 file keeps importing into + # a timestamp column as raw microseconds, exactly as it did before. + check "control: a non-temporal tag is not caught by the temporal refusal" \ + "$(tu_import_state tu_int64_ts timestamp)" "00000" + + # ---- the unit on a nested field --------------------------------------- + check "a list element's nanosecond unit is honoured, not just the top level" \ + "$(tu_value tu_list_ns 'timestamp[]')" "{\"2000-01-01 00:00:00\"}" + + psql_run "DROP TYPE IF EXISTS tu_st CASCADE; CREATE TYPE tu_st AS (a int, b timestamp);" \ + >/dev/null 2>&1 + check "a struct field's nanosecond unit is honoured at child index 1" \ + "$(tu_value tu_struct_ns tu_st)" "(7,\"2000-01-01 00:00:00\")" fi IXFILE="$PGC_WORKDIR/ix_roundtrip.arrows" From d93f0cd5039e911e398001bcb03d58499ca539c4 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Tue, 1 Sep 2026 19:32:45 +0000 Subject: [PATCH 2/2] docs: split two sentences over the plain-language word limit test/docs_style.sh measures sentence length and refuses anything over 25 words. Two sentences added to docs/sql-reference.md in the previous commit were 32 and 26 words, and the suite matrix caught them on PG17 and PG18. They say the same thing in three sentences and two. --- docs/sql-reference.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/sql-reference.md b/docs/sql-reference.md index 431c662e..5b0d26d9 100644 --- a/docs/sql-reference.md +++ b/docs/sql-reference.md @@ -476,9 +476,9 @@ Inserts the rows of an Arrow IPC stream file at `path` into the existing table rows inserted. Temporal columns are read in the unit the file declares, not the unit -`export_arrow` writes. A `date` column accepts `date32` and `date64`; a `time` -column accepts `time32` in seconds or milliseconds and `time64` in microseconds -or nanoseconds; a `timestamp` column accepts any of the four `Timestamp` units. +`export_arrow` writes. A `date` column accepts `date32` and `date64`. A `time` +column accepts `time32` in seconds or milliseconds, and `time64` in microseconds +or nanoseconds. A `timestamp` column accepts any of the four `Timestamp` units. Values are converted to PostgreSQL's own units, and a value that cannot be represented is refused with `22008` rather than stored wrong. @@ -487,7 +487,7 @@ stores. Narrowing floors, so an instant before 1970 reports the microsecond it falls in. Sub-microsecond precision is lost; nothing else is. A temporal Arrow type the target column cannot hold is refused with `42804` -rather than read: a `time64` file does not import into a `date` column. A file +rather than read. A `time64` file does not import into a `date` column. A file whose type is not temporal is unaffected. ### pgcolumnar.import_parquet(rel regclass, path text) returns bigint