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..5b0d26d9 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"