From 2621e1bb474e3dffd6e22848ef2c93cde2e82288 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 22 Oct 2026 18:40:38 +0000 Subject: [PATCH 1/7] fix: a Parquet timestamp read into the other timestamp type follows PostgreSQL's casts (#1706) Item 2 of #1706, decided there as option (a). The reader skipped the TimestampType's isAdjustedToUTC flag and returned the raw microseconds for either target type. In a New York session a naive (wall-time) column read into timestamptz came back as 08:00-04 instead of 12:00-04, and an adjusted column read into timestamp kept its UTC wall time instead of 08:00. The reader now keeps the flag (the legacy TIMESTAMP_* converted types are defined as adjusted) and, in those two mixed cases only, converts as timestamp_timestamptz and timestamptz_timestamp do, in the session zone. It is written from timestamp2tm, DetermineTimeZoneOffset and tm2timestamp, so it is the same on every major: PG19 renamed the exported helper, and PG15-18 export none for timestamptz -> timestamp. Same-kind reads, and any read in a UTC session, are unchanged. A wall time is not monotonic in its instant across a spring-forward gap: in New York on 2024-03-10, 02:00, 02:30 and 03:00 become 07:00Z, 07:30Z and 07:00Z. Converted row-group statistics need not bound the converted rows, so the foreign table no longer skips row groups of a column read this way. Measured: with skipping left on, `t > '07:15Z'` over such a group returned 0 rows instead of 1. The Iceberg foreign table's year(), month(), day() and hour() pruning compared a constant with partition cells the writer computed in the source field's own kind, while the reader now converts a mixed-kind column's rows. PgColumnarIcebergTemporalMap now leaves out a field whose Iceberg source type is the other timestamp kind than its column, so nothing prunes on it. Measured on PG17 before the guard, in New York: byhour declared timestamptz with `ts >= '2021-03-01 06:00+00'` returned no rows where iceberg_scan returned 2,3, and byhour_tz declared timestamp with `tt <= '2021-02-28 20:00'` returned none where it returned 1,2. #1707 (merged) is a prerequisite: with the reader honouring the flag and the old exporter marking timestamp adjusted, a timestamp exported and re-imported in a New York session came back 08:00 (measured). An arm pins that round trip. New suite test/parquet_timestamp_zone.sh and its twin, 16 checks, including the naive file read in a second zone (Tokyo), the repeated hour of the 2024-11-03 fall-back, and test/fixtures/parquet/legacy_timestamp_micros.parquet, written by main's exporter before #1707 (0c5e924d) with only the TIMESTAMP_MICROS converted type. Two arms in test/iceberg_fdw.sh and its twin. docs/user-guide.md states the rule; CHANGELOG under Changed. Measured on PG17: - Red, the head before the Iceberg guard: both new iceberg_fdw checks, in shell and twin ("got [] want [2,3]", "got [] want [1,2]"). - Mutants, the same names in shell and twin: the flag ignored (6), no instant-to-wall shift (2), skipping on converted stats (1), main's exporter before #1707 (the round trip, 1), the conversion pinned to New York (the Tokyo pair, 2), legacy converted types read as naive (the legacy pair, 2), the zone offset taken at the wall time read as UTC (the fall-back hour and the two gap checks, 3), no Iceberg kind guard (2). - Green: parquet_timestamp_zone 16/16 and iceberg_fdw 76/76, shell; 16/16 and 80/80, twin; on each of PG15 to PG19. - Without pyarrow the twin declines by the shell's skip name. - docs_style 53/53, harness_selftest 1248/1248, shellcheck clean. Ledger: 6951 rows, 2811 dated by the mutations and 4140 never. suites_not_covered 75 -> 75 (a new suite, registered and covered together; iceberg_fdw is not yet seeded, so its two new checks add no rows), cluster_tests 3222 -> 3240, TESTS.md section 296. The ledger gate, guards (804 passed), name grading and the self-test pass over 9ab6a4cf. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 13 + docs/user-guide.md | 18 ++ src/columnar_iceberg.c | 54 +++- src/columnar_parquet_reader.c | 75 ++++- test/check_ledger.tsv | 16 ++ test/check_ledger_budget.txt | 20 +- .../parquet/legacy_timestamp_micros.parquet | Bin 0 -> 146 bytes test/iceberg_fdw.sh | 25 ++ test/parquet_timestamp_zone.sh | 139 ++++++++++ test/pytest/TESTS.md | 27 ++ test/pytest/expected_tests.txt | 4 +- test/pytest/test_compare_to_bash.py | 1 + test/pytest/test_iceberg_fdw.py | 48 ++++ test/pytest/test_parquet_timestamp_zone.py | 261 ++++++++++++++++++ test/run_all_versions.sh | 1 + 15 files changed, 697 insertions(+), 5 deletions(-) create mode 100644 test/fixtures/parquet/legacy_timestamp_micros.parquet create mode 100755 test/parquet_timestamp_zone.sh create mode 100644 test/pytest/test_parquet_timestamp_zone.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d1baa30c..1db70a33d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -236,6 +236,19 @@ true until the next version shipped. over a failed attempt's file is allowed. ### Changed +- **A Parquet timestamp read into the other PostgreSQL timestamp type is + converted in the session's time zone.** The reader ignored the file's + `isAdjustedToUTC` flag. A wall-time column read into `timestamptz` was taken as + UTC (12:00 read as 08:00-04 in a New York session), and a UTC-adjusted column + read into `timestamp` kept its UTC wall time. Both now follow PostgreSQL's casts. + Same-type reads are unchanged, and so is any read in a UTC session (#1706). + A legacy `TIMESTAMP_MILLIS` or `TIMESTAMP_MICROS` column with no logical type, + as pgcolumnar's own exporter wrote before #1707, is read as UTC-adjusted, as + the Parquet format defines it. The Iceberg foreign table no longer prunes on a + `year()`, `month()`, `day()` or `hour()` partition whose source column is the + other timestamp type: its partition values are in the source's type, so they + no longer bound the converted rows. + `test/parquet_timestamp_zone.sh`, `test/iceberg_fdw.sh` and their twins. - `GROUP BY` is vectorized by default (#1292). `pgcolumnar.enable_group_vectorization` is now `on`. The planner takes the diff --git a/docs/user-guide.md b/docs/user-guide.md index cf8252056..e78b40614 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -245,6 +245,24 @@ EXPLAIN (ANALYZE, COSTS OFF) SELECT id FROM events_parquet WHERE ts >= '2026-01- -- Files: 4 ``` +**Timestamps and time zones.** A Parquet timestamp column holds either instants +(UTC-adjusted) or wall times with no zone. Read into the matching type +(`timestamptz` for instants, `timestamp` for wall times), a value comes back +unchanged. Read into the other type, it is converted as PostgreSQL's own cast +converts it, in the session's `TimeZone`: + +- a wall time read into `timestamptz` is taken as local time in that zone, like + `'2024-06-01 12:00'::timestamp::timestamptz`; +- an instant read into `timestamp` becomes the local wall time, like + `'2024-06-01 12:00+00'::timestamptz::timestamp`. + +A foreign-table scan does not skip row groups of a column read this way, because +the conversion does not keep order across a daylight-saving change. For the same +reason the Iceberg foreign table does not prune files on a `year()`, `month()`, +`day()` or `hour()` partition of such a column. A column marked only with the +older `TIMESTAMP_MILLIS` or `TIMESTAMP_MICROS` type, with no `isAdjustedToUTC` +flag, holds instants: the Parquet format defines it so. + The `path` may also be an object-storage URL. Every read and export function, and both foreign-data wrappers, accept an `s3://`, `http://`, or `https://` URL where they accept a local path. diff --git a/src/columnar_iceberg.c b/src/columnar_iceberg.c index c7deae891..dd4b2732b 100644 --- a/src/columnar_iceberg.c +++ b/src/columnar_iceberg.c @@ -1116,6 +1116,50 @@ ice_field_id_by_name(JsonbContainer *fieldsc, uint32 nf, const char *nm) return -1; } +/* + * Whether the current-schema field named `nm` is a timestamp of the OTHER kind + * than the PostgreSQL column `typid` (timestamp vs timestamptz, any unit). The + * reader converts such a column's rows as PostgreSQL's casts do, in the session + * zone (#1706), but the writer computed its temporal partition cells in the + * source's own kind, so those cells do not bound the converted rows. + */ +static bool +ice_field_timestamp_kind_differs(JsonbContainer *fieldsc, uint32 nf, + const char *nm, Oid typid) +{ + uint32 j; + + if (typid != TIMESTAMPOID && typid != TIMESTAMPTZOID) + return false; + for (j = 0; j < nf; j++) + { + JsonbValue *sf = getIthJsonbValueFromContainer(fieldsc, j); + JsonbValue *v; + bool src_tz; + + if (sf == NULL || sf->type != jbvBinary) + continue; + v = ice_field(sf->val.binary.data, "name"); + if (v == NULL || v->type != jbvString || + (int) strlen(nm) != v->val.string.len || + strncmp(nm, v->val.string.val, v->val.string.len) != 0) + continue; + v = ice_field(sf->val.binary.data, "type"); + if (v == NULL || v->type != jbvString) + return false; + if (v->val.string.len >= 11 && + strncmp(v->val.string.val, "timestamptz", 11) == 0) + src_tz = true; + else if (v->val.string.len >= 9 && + strncmp(v->val.string.val, "timestamp", 9) == 0) + src_tz = false; + else + return false; + return src_tz != (typid == TIMESTAMPTZOID); + } + return false; +} + /* * Map the current partition spec's IDENTITY fields to foreign-table columns, for * FDW pruning. On return *out_attno[k] is the 1-based attno of the table column @@ -1581,7 +1625,10 @@ PgColumnarIcebergDayMap(const char *path, const PgColumnarObjStoreConfig *cfg, * default spec's partition-cell positions and 1-based source attnos for the * fields carrying that transform. The FDW routes each mapped field by its source * column type: the coarse temporal compiler handles the timestamp cases, the - * exact day() compiler handles a day() on a date. + * exact day() compiler handles a day() on a date. A field whose source is a + * timestamp of the other kind than its column (timestamp vs timestamptz) is left + * out, so nothing prunes on it: its cells are in the source's kind, its rows are + * converted in the session zone (#1706). */ void PgColumnarIcebergTemporalMap(const char *path, const PgColumnarObjStoreConfig *cfg, @@ -1676,6 +1723,11 @@ PgColumnarIcebergTemporalMap(const char *path, const PgColumnarObjStoreConfig *c continue; if (ice_field_id_by_name(fieldsc, nf, NameStr(att->attname)) == srcid) { + /* the cells are in the source's kind, the rows in the column's */ + if (ice_field_timestamp_kind_differs(fieldsc, nf, + NameStr(att->attname), + att->atttypid)) + break; pos[cnt] = (int) k; attno[cnt] = a + 1; cnt++; diff --git a/src/columnar_parquet_reader.c b/src/columnar_parquet_reader.c index 42d8f15fc..16d0a5991 100644 --- a/src/columnar_parquet_reader.c +++ b/src/columnar_parquet_reader.c @@ -76,6 +76,8 @@ #include "utils/rel.h" #include "utils/snapmgr.h" #include "utils/timestamp.h" +#include "utils/datetime.h" +#include "pgtime.h" #include "utils/varlena.h" #include "utils/tuplestore.h" #include "utils/typcache.h" @@ -137,6 +139,7 @@ typedef struct PqSchemaCol int converted_type; /* -1 if none */ int time_unit; /* PQ_TU_* for a TIME/TIMESTAMP column */ bool is_timestamp; /* the unit belongs to TIMESTAMP, not TIME */ + bool adjusted_utc; /* TIMESTAMP isAdjustedToUTC: an instant, not a wall time */ int type_length; /* FIXED_LEN_BYTE_ARRAY length */ int scale; /* DECIMAL scale (0 if none) */ int precision; /* DECIMAL precision (0 if none) */ @@ -416,8 +419,12 @@ parse_logical_type(TCReader *r, PqSchemaCol *sc) PgColumnarThriftField(r, &uft, &ufid, &uLast); } } + else if (ifid == 1 && isTs && + (ift == TC_BOOL_TRUE || ift == TC_BOOL_FALSE)) + /* isAdjustedToUTC: compact protocol carries a bool in its type */ + sc->adjusted_utc = (ift == TC_BOOL_TRUE); else - PgColumnarThriftSkip(r, ift); /* isAdjustedToUTC and anything later */ + PgColumnarThriftSkip(r, ift); /* anything later */ } } else @@ -436,6 +443,8 @@ parse_schema_element(TCReader *r, PqSchemaCol *sc) sc->converted_type = -1; sc->time_unit = PQ_TU_NONE; sc->is_timestamp = false; + /* the legacy TIMESTAMP_* converted types are defined as UTC-adjusted */ + sc->adjusted_utc = true; sc->type_length = 0; sc->scale = 0; sc->precision = 0; @@ -875,8 +884,21 @@ typedef struct PqColPlan int type_length; /* FIXED_LEN_BYTE_ARRAY width, for uuid/decimal */ int dec_scale; /* DECIMAL scale, when the target is numeric */ bool is_decimal; /* the bytes are a DECIMAL unscaled integer */ + int ts_shift; /* PQ_TS_*: a cast between wall time and instant */ } PqColPlan; +/* + * A Parquet timestamp read into the OTHER PostgreSQL timestamp type is converted + * as PostgreSQL's own cast would, in the session zone (#1706): a naive (not + * UTC-adjusted) wall time into timestamptz, and an adjusted instant into + * timestamp. The same-kind reads are left as they are. + */ +#define PQ_TS_NONE 0 +#define PQ_TS_WALL_TO_INSTANT 1 +#define PQ_TS_INSTANT_TO_WALL 2 + +static bool pq_ts_shift(int shift, Timestamp in, Timestamp *out); + /* * Convert a stored TIME/TIMESTAMP value to the microseconds PostgreSQL stores, * per the column's declared unit. Returns false if the conversion overflows. @@ -1176,7 +1198,9 @@ plain_value_to_datum(const PqColPlan *plan, const uint8 **p, const uint8 *end, if (!pq_scale_to_usecs(plan->time_unit, v, &us) || pg_sub_s64_overflow(us, PG_TO_UNIX_USECS, &t) || - !IS_VALID_TIMESTAMP((Timestamp) t)) + !IS_VALID_TIMESTAMP((Timestamp) t) || + (plan->ts_shift != PQ_TS_NONE && + !pq_ts_shift(plan->ts_shift, (Timestamp) t, (Timestamp *) &t))) return pq_value_out_of_range(strict, ok, ERRCODE_DATETIME_FIELD_OVERFLOW, tn, v); @@ -2276,6 +2300,47 @@ pq_plan_bind_schema(PqColPlan *plan, const PqSchemaCol *sc) plan->dec_scale = sc->scale; plan->is_decimal = (plan->typid == NUMERICOID && sc->converted_type == PQ_CT_DECIMAL); + plan->ts_shift = PQ_TS_NONE; + if (sc->is_timestamp && !sc->adjusted_utc && plan->typid == TIMESTAMPTZOID) + plan->ts_shift = PQ_TS_WALL_TO_INSTANT; + else if (sc->is_timestamp && sc->adjusted_utc && plan->typid == TIMESTAMPOID) + plan->ts_shift = PQ_TS_INSTANT_TO_WALL; +} + +/* + * The two casts, from the primitives PostgreSQL's timestamp_timestamptz and + * timestamptz_timestamp use, so they behave the same on every major (PG19 renamed + * the exported helper, and PG15-18 export no timestamptz -> timestamp one). + * false when the result is out of range. + */ +static bool +pq_ts_shift(int shift, Timestamp in, Timestamp *out) +{ + struct pg_tm tm; + fsec_t fsec; + int tz; + + if (TIMESTAMP_NOT_FINITE(in)) + { + *out = in; + return true; + } + if (shift == PQ_TS_WALL_TO_INSTANT) + { + if (timestamp2tm(in, NULL, &tm, &fsec, NULL, NULL) != 0) + return false; + tz = DetermineTimeZoneOffset(&tm, session_timezone); + if (tm2timestamp(&tm, fsec, &tz, out) != 0) + return false; + } + else + { + if (timestamp2tm(in, &tz, &tm, &fsec, NULL, NULL) != 0) + return false; + if (tm2timestamp(&tm, fsec, NULL, out) != 0) + return false; + } + return IS_VALID_TIMESTAMP(*out); } /* @@ -4486,6 +4551,12 @@ pqfdw_clause_excludes_group(ImpLeaf *leaf, PqChunk *ch, int strategy, return false; if (con->consttype != leaf->plan.typid || con->constisnull) return false; + /* + * A wall time is not monotonic in its instant across a spring-forward gap, so + * converted statistics need not bound the converted rows (#1706): never skip. + */ + if (leaf->plan.ts_shift != PQ_TS_NONE) + return false; tce = lookup_type_cache(leaf->plan.typid, TYPECACHE_CMP_PROC_FINFO); if (!OidIsValid(tce->cmp_proc_finfo.fn_oid)) diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index e5e80d92d..ace68abc1 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -5804,6 +5804,22 @@ parquet_nested_import parquet_nested_import nested import round-trips (arrays + parquet_nested_import parquet_nested_import pyarrow import rows 15;16;17;18;19 never - parquet_nested_import parquet_nested_import pyarrow nested file matches oracle 15;16;17;18;19 2026-10-21 columnar_parquet_reader.c: an empty Parquet list is read back as a NULL array parquet_nested_import parquet_nested_import round-trips after delete 15;16;17;18;19 2026-10-21 columnar_parquet_reader.c: an empty Parquet list is read back as a NULL array +parquet_timestamp_zone parquet_timestamp_zone a legacy TIMESTAMP_MICROS file read into timestamp is the session-zone wall time of its instant 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: a column with only a legacy TIMESTAMP_* converted type reads as not UTC-adjusted +parquet_timestamp_zone parquet_timestamp_zone a legacy TIMESTAMP_MICROS file read into timestamptz keeps its instant 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: a column with only a legacy TIMESTAMP_* converted type reads as not UTC-adjusted +parquet_timestamp_zone parquet_timestamp_zone a naive file read into timestamptz in Tokyo is its wall time there, as the cast gives 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: a naive column read into timestamptz is converted in America/New_York, not the session zone +parquet_timestamp_zone parquet_timestamp_zone a naive file read into timestamptz is its wall time in the session zone 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: isAdjustedToUTC is ignored, so every timestamp column reads as UTC-adjusted +parquet_timestamp_zone parquet_timestamp_zone a naive wall time in the fall-back hour read into timestamptz is what the cast gives 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: a naive wall time takes the zone offset in force at that wall time read as UTC, not at the wall time itself +parquet_timestamp_zone parquet_timestamp_zone a row group spanning a spring-forward gap is not skipped on its converted min and max 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: a converted timestamp column's row groups are skipped on their converted min and max;columnar_parquet_reader.c: a naive wall time takes the zone offset in force at that wall time read as UTC, not at the wall time itself +parquet_timestamp_zone parquet_timestamp_zone an adjusted file read into timestamp is the session-zone wall time of its instant 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: an adjusted column read into timestamp is not converted to the session-zone wall time +parquet_timestamp_zone parquet_timestamp_zone an export and import round trip keeps timestamp and timestamptz in a non-UTC session 15;16;17;18;19 2026-10-22 columnar_parquet.c: main's exporter before #1707, which marks a timestamp column UTC-adjusted +parquet_timestamp_zone parquet_timestamp_zone and that is what PostgreSQL's cast from timestamp gives 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: isAdjustedToUTC is ignored, so every timestamp column reads as UTC-adjusted +parquet_timestamp_zone parquet_timestamp_zone and that is what PostgreSQL's cast from timestamptz gives 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: an adjusted column read into timestamp is not converted to the session-zone wall time +parquet_timestamp_zone parquet_timestamp_zone and the Tokyo reading is a different instant from the New York one 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: a naive column read into timestamptz is converted in America/New_York, not the session zone +parquet_timestamp_zone parquet_timestamp_zone control: a naive file read into timestamp keeps its wall time 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: isAdjustedToUTC is ignored, so every timestamp column reads as UTC-adjusted +parquet_timestamp_zone parquet_timestamp_zone control: an adjusted file read into timestamptz keeps its instant 15;16;17;18;19 never - +parquet_timestamp_zone parquet_timestamp_zone premise: pyarrow wrote the naive file without isAdjustedToUTC and the other with it 15;16;17;18;19 never - +parquet_timestamp_zone parquet_timestamp_zone premise: the legacy fixture carries the TIMESTAMP_MICROS converted type and no logical type 15;16;17;18;19 never - +parquet_timestamp_zone parquet_timestamp_zone the gap file's wall times 02:00, 02:30 and 03:00 become, sorted, the instants 07:00Z, 07:00Z and 07:30Z 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: a naive wall time takes the zone offset in force at that wall time read as UTC, not at the wall time itself;columnar_parquet_reader.c: isAdjustedToUTC is ignored, so every timestamp column reads as UTC-adjusted partition_lifecycle partition_lifecycle a NULL cutoff is refused, in both overloads 15;16;17;18;19 2026-10-13 partition_drop_plan without its NULL-cutoff check, in both overloads partition_lifecycle partition_lifecycle a call that would create more than 1000 partitions is refused before any DDL 15;16;17;18;19 2026-10-12 partition_create_ahead without the 1000-partition bound partition_lifecycle partition_lifecycle a cutoff before every partition plans nothing 15;16;17;18;19 2026-10-13 the cutoff filter removed: every partition planned diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 8b62c6794..b304ad7de 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -7056,4 +7056,22 @@ suites_not_covered 27 # never 4520 + 15 = 4535 # dated 3443 + 2 = 3445 # 4535 + 3445 == 7980 -checks_never_observed_red 4535 +# +# #1706 item 2, parquet_timestamp_zone seeded, with its twin (jdatcmd): 4535 -> 4538. A new +# suite, so suites_not_covered does not move: it is registered and covered in the same change. +# It enters with 16 checks from five-major runs (all green on each of 15-19, shell and twin +# alike). 13 are dated by seven mutations, each reddening the same names in both twins: the +# reader ignoring isAdjustedToUTC (6), not converting an adjusted column read into +# timestamp (2), skipping a converted column's row groups on their converted statistics +# (the spring-forward group, 1), main's exporter before #1707 (the round trip, 1), the +# conversion pinned to New York instead of the session zone (the Tokyo pair, 2), a legacy +# TIMESTAMP_MICROS column read as naive (the legacy pair, 2), and the zone offset taken at +# the wall time read as UTC (the fall-back hour, with the two gap checks, 1 new). The two +# premises and the adjusted-into-timestamptz control arrive never. iceberg_fdw's two new +# mixed-kind hour() checks add no rows: that suite is not yet seeded. +# +# ledger rows 7980 + 16 = 7996 +# never 4535 + 3 = 4538 +# dated 3445 + 13 = 3458 +# 4538 + 3458 == 7996 +checks_never_observed_red 4538 diff --git a/test/fixtures/parquet/legacy_timestamp_micros.parquet b/test/fixtures/parquet/legacy_timestamp_micros.parquet new file mode 100644 index 0000000000000000000000000000000000000000..8c842e2ffa07b1068c5996762d3050a50fc3c620 GIT binary patch literal 146 zcmWG=3^EjD5S0;?(Gg_=GT21f7#Nrs7#Nrt87{oNc1e= TIMESTAMPTZ '2021-03-01 01:00:00+00'")" fi +# ---- a temporal transform read into the OTHER timestamp type (#1706) --------- +# The reader converts a naive (timestamp) source read into timestamptz, and a +# timestamptz source read into timestamp, as PostgreSQL's casts do in the session +# zone. The writer computed each partition cell in the source's own kind, so a +# cell does not bound the converted rows, and the FDW must not prune on it. +# In New York, byhour's naive 2021-03-01 00:00, 01:00, 05:00 are 05:00Z, 06:00Z +# and 10:00Z; byhour_tz's 00:00Z, 01:00Z, 05:00Z are the wall times 2021-02-28 +# 19:00, 20:00 and 2021-03-01 00:00. Each FDW read is compared with iceberg_scan +# under the same declaration and zone, and check_text refuses two empty readings. +qny() { PGOPTIONS="-c TimeZone=America/New_York" q "$1"; } +if [ -n "${MD_th:-}" ] && ftmpx hn byhour_tz "tt timestamp"; then + q "CREATE FOREIGN TABLE hw (id bigint, ts timestamptz) + SERVER ice OPTIONS (metadata_path '$MD_th')" >/dev/null + check_text "hour() on a naive source read as timestamptz in New York matches iceberg_scan" \ + "$(qny "SELECT string_agg(id::text, ',' ORDER BY id) FROM hw WHERE ts >= TIMESTAMPTZ '2021-03-01 06:00:00+00'")" \ + "$(qny "SELECT string_agg(id::text, ',' ORDER BY id) + FROM pgcolumnar.iceberg_scan('$MD_th') AS t(id bigint, ts timestamptz) + WHERE ts >= TIMESTAMPTZ '2021-03-01 06:00:00+00'")" + check_text "hour() on a timestamptz source read as timestamp in New York matches iceberg_scan" \ + "$(qny "SELECT string_agg(id::text, ',' ORDER BY id) FROM hn WHERE tt <= TIMESTAMP '2021-02-28 20:00:00'")" \ + "$(qny "SELECT string_agg(id::text, ',' ORDER BY id) + FROM pgcolumnar.iceberg_scan('$MD_hn') AS t(id bigint, tt timestamp) + WHERE tt <= TIMESTAMP '2021-02-28 20:00:00'")" +fi + # ---- an invalid option is rejected by the validator ------------------------- check "an unknown table option is refused (FDW_INVALID_OPTION_NAME)" \ "$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -qtA 2>&1 </dev/null; then + pgc_skip pyarrow "pyarrow not available; Parquet timestamp zone suite needs it" +fi + +W="$PGC_WORKDIR" +# Every session of this suite runs in New York, so each reading below is in that zone. +export PGOPTIONS="-c TimeZone=America/New_York" +psql_run "CREATE SERVER pq FOREIGN DATA WRAPPER pgcolumnar_parquet;" + +python3 - "$W" <<'PY' +import sys, datetime as dt, pyarrow as pa, pyarrow.parquet as pq +w = sys.argv[1] +noon = dt.datetime(2024, 6, 1, 12, 0) +# naive: timestamp[us], isAdjustedToUTC=false; adjusted: tz=UTC, isAdjustedToUTC=true +pq.write_table(pa.table({"t": pa.array([noon], pa.timestamp("us"))}), f"{w}/naive.parquet") +pq.write_table(pa.table({"t": pa.array([noon], pa.timestamp("us", tz="UTC"))}), f"{w}/adjusted.parquet") +# one row group spanning New York's 2024-03-10 spring-forward gap +gap = [dt.datetime(2024, 3, 10, 2, 0), dt.datetime(2024, 3, 10, 2, 30), dt.datetime(2024, 3, 10, 3, 0)] +pq.write_table(pa.table({"t": pa.array(gap, pa.timestamp("us"))}), f"{w}/gap.parquet", row_group_size=3) +# the repeated hour of New York's 2024-11-03 fall-back +fb = [dt.datetime(2024, 11, 3, 1, 30)] +pq.write_table(pa.table({"t": pa.array(fb, pa.timestamp("us"))}), f"{w}/fallback.parquet") +PY +cp "$(dirname "${BASH_SOURCE[0]}")/fixtures/parquet/legacy_timestamp_micros.parquet" "$W/legacy.parquet" +for f in naive adjusted gap fallback legacy; do chmod 644 "$W/$f.parquet"; done + +check "premise: pyarrow wrote the naive file without isAdjustedToUTC and the other with it" \ + "$(python3 -c " +import sys, pyarrow.parquet as pq +a = lambda f: pq.ParquetFile(f).schema_arrow.field('t').type.tz +print(a('$W/naive.parquet'), a('$W/adjusted.parquet'))")" "None UTC" +# pyarrow derives a logical type from a converted type and says so in this flag +check "premise: the legacy fixture carries the TIMESTAMP_MICROS converted type and no logical type" \ + "$(python3 -c " +import pyarrow.parquet as pq +c = pq.ParquetFile('$W/legacy.parquet').metadata.schema.column(0) +print(c.converted_type, 'is_from_converted_type=true' in str(c.logical_type))")" "TIMESTAMP_MICROS True" + +psql_run "CREATE TABLE n_ts (t timestamp) USING pgcolumnar; CREATE TABLE n_tz (t timestamptz) USING pgcolumnar; + CREATE TABLE a_ts (t timestamp) USING pgcolumnar; CREATE TABLE a_tz (t timestamptz) USING pgcolumnar;" +for t in n_ts n_tz; do psql_run "SELECT pgcolumnar.import_parquet('$t', '$W/naive.parquet');"; done +for t in a_ts a_tz; do psql_run "SELECT pgcolumnar.import_parquet('$t', '$W/adjusted.parquet');"; done + +check_text "a naive file read into timestamptz is its wall time in the session zone" \ + "$(q "SELECT t::text FROM n_tz;")" "2024-06-01 12:00:00-04" +check_text "and that is what PostgreSQL's cast from timestamp gives" \ + "$(q "SELECT (SELECT t FROM n_tz) = '2024-06-01 12:00'::timestamp::timestamptz;")" "t" +check_text "an adjusted file read into timestamp is the session-zone wall time of its instant" \ + "$(q "SELECT t::text FROM a_ts;")" "2024-06-01 08:00:00" +check_text "and that is what PostgreSQL's cast from timestamptz gives" \ + "$(q "SELECT (SELECT t FROM a_ts) = '2024-06-01 12:00+00'::timestamptz::timestamp;")" "t" +check_text "control: a naive file read into timestamp keeps its wall time" \ + "$(q "SELECT t::text FROM n_ts;")" "2024-06-01 12:00:00" +check_text "control: an adjusted file read into timestamptz keeps its instant" \ + "$(q "SELECT t::text FROM a_tz;")" "2024-06-01 08:00:00-04" + +# The round trip through pgcolumnar's own export, in the same zone. This holds only +# because the exporter writes timestamp as naive (#1707): with the reader honouring +# the flag and the old exporter marking timestamp adjusted, the timestamp came back +# 08:00, four hours early. +psql_run "CREATE TABLE rt_src (ts timestamp, tz timestamptz) USING pgcolumnar; + INSERT INTO rt_src VALUES ('2024-06-01 12:00', '2024-06-01 12:00'); + CREATE TABLE rt_dst (ts timestamp, tz timestamptz) USING pgcolumnar;" +psql_run "SELECT pgcolumnar.export_parquet('rt_src', '$W/rt.parquet');" +psql_run "SELECT pgcolumnar.import_parquet('rt_dst', '$W/rt.parquet');" +check_text "an export and import round trip keeps timestamp and timestamptz in a non-UTC session" \ + "$(q "SELECT ts::text || ' | ' || tz::text FROM rt_dst;")" "2024-06-01 12:00:00 | 2024-06-01 12:00:00-04" + +psql_run "CREATE FOREIGN TABLE f_gap (t timestamptz) SERVER pq OPTIONS (path '$W/gap.parquet');" +check_text "the gap file's wall times 02:00, 02:30 and 03:00 become, sorted, the instants 07:00Z, 07:00Z and 07:30Z" \ + "$(q "SELECT string_agg(to_char(t AT TIME ZONE 'UTC', 'HH24:MI'), ',' ORDER BY t) FROM f_gap;")" \ + "07:00,07:00,07:30" +check_text "a row group spanning a spring-forward gap is not skipped on its converted min and max" \ + "$(q "SELECT count(*) FROM f_gap WHERE t > '2024-03-10 07:15+00';")" "1" + +# A second zone: the same naive file read through a foreign table in Tokyo. +psql_run "CREATE FOREIGN TABLE f_naive (t timestamptz) SERVER pq OPTIONS (path '$W/naive.parquet');" +qtk() { PGOPTIONS="-c TimeZone=Asia/Tokyo" q "$1"; } +check_text "a naive file read into timestamptz in Tokyo is its wall time there, as the cast gives" \ + "$(qtk "SELECT t::text || ' ' || (t = '2024-06-01 12:00'::timestamp::timestamptz)::text FROM f_naive;")" \ + "2024-06-01 12:00:00+09 true" +check_text "and the Tokyo reading is a different instant from the New York one" \ + "$(qtk "SELECT ((SELECT t FROM f_naive) <> (SELECT t FROM n_tz))::text;")" "true" + +# The repeated hour: 01:30 happens twice in New York on 2024-11-03. +psql_run "CREATE FOREIGN TABLE f_fb (t timestamptz) SERVER pq OPTIONS (path '$W/fallback.parquet');" +check_text "a naive wall time in the fall-back hour read into timestamptz is what the cast gives" \ + "$(q "SELECT t::text || ' ' || (t = '2024-11-03 01:30'::timestamp::timestamptz)::text FROM f_fb;")" \ + "2024-11-03 01:30:00-05 true" + +# The legacy converted type, read as the instant the spec defines it to be. +psql_run "CREATE FOREIGN TABLE f_leg_ts (t timestamp) SERVER pq OPTIONS (path '$W/legacy.parquet'); + CREATE FOREIGN TABLE f_leg_tz (t timestamptz) SERVER pq OPTIONS (path '$W/legacy.parquet');" +check_text "a legacy TIMESTAMP_MICROS file read into timestamp is the session-zone wall time of its instant" \ + "$(q "SELECT t::text || ' ' || (t = '2024-06-01 12:00+00'::timestamptz::timestamp)::text FROM f_leg_ts;")" \ + "2024-06-01 08:00:00 true" +check_text "a legacy TIMESTAMP_MICROS file read into timestamptz keeps its instant" \ + "$(q "SELECT t::text FROM f_leg_tz;")" "2024-06-01 08:00:00-04" + +pgc_summary diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 6048fbf14..d2a4db9c0 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -388,6 +388,7 @@ behaviour, the source of that number is named. - [340. test_decode_skip_interrupts.py: the Thrift and Avro skip loops of a hostile file yield to a cancel, per skipped value](#340-test_decode_skip_interruptspy-the-thrift-and-avro-skip-loops-of-a-hostile-file-yield-to-a-cancel-per-skipped-value) - [341. test_native_cancel.py: a scan's row-group load yields to statement_timeout inside the decode loop](#341-test_native_cancelpy-a-scans-row-group-load-yields-to-statement_timeout-inside-the-decode-loop) - [342. test_objstore_export_double_cancel.py: a second cancel during a remote export's cleanup keeps the export's own error](#342-test_objstore_export_double_cancelpy-a-second-cancel-during-a-remote-exports-cleanup-keeps-the-exports-own-error) +- [343. test_parquet_timestamp_zone.py: a Parquet timestamp read into the other timestamp type follows PostgreSQL's casts](#343-test_parquet_timestamp_zonepy-a-parquet-timestamp-read-into-the-other-timestamp-type-follows-postgresqls-casts) ## 1. How to read a test in here @@ -4445,6 +4446,8 @@ carry are `pgc_skip`'s refusal names, which are structural and declared in | `test_the_year_table_reads_whole_and_keeps_the_boundary_file` | the boundary case stated alone | | `test_a_year_equality_prunes_to_one_file` | an equality narrows to the constant's own bucket | | `test_a_year_on_date_equality_prunes_to_one_file` | the same on a date column | +| `test_an_hour_partition_over_a_naive_source_read_as_timestamptz_keeps_its_rows` | #1706: `byhour` (a naive source) declared `timestamptz` in New York returns `iceberg_scan`'s rows; its hour cells are wall times, so the FDW must not prune on them | +| `test_an_hour_partition_over_a_timestamptz_source_read_as_timestamp_keeps_its_rows` | #1706: the reverse, `byhour_tz` declared `timestamp` | | `test_an_unknown_table_option_is_refused` | the validator, by SQLSTATE `HV00D` rather than by message text | | `test_a_plan_with_no_pruning_marker_is_not_read_as_zero` | a plan that never mentions `Files Pruned` is not read as 0; needs no server | @@ -12845,3 +12848,27 @@ Every test reads one value from a module fixture that runs the whole scenario on | `test_no_completed_object_remains_at_the_prefix` | double cancel: no completed object remains at the prefix | | `test_the_server_survived` | double cancel: the server survived | +## 343. test_parquet_timestamp_zone.py: a Parquet timestamp read into the other timestamp type follows PostgreSQL's casts + +The pytest twin of `test/parquet_timestamp_zone.sh` (#1706 item 2), every check by the shell's name. In a New York session: a naive Parquet timestamp read into `timestamptz` is its wall time in the session zone, and a UTC-adjusted one read into `timestamp` is the session-zone wall time of its instant, each equal to PostgreSQL's own cast; the same-kind reads are controls; an export and import round trip keeps both types (with #1707's exporter); and a row group spanning the 2024-03-10 spring-forward gap is not skipped on its converted min and max, which need not bound its converted rows. The naive file is read again in Tokyo, so a reader that hard-codes a zone fails; the repeated hour of the 2024-11-03 fall-back resolves as the cast resolves it; and `test/fixtures/parquet/legacy_timestamp_micros.parquet`, written by pgcolumnar's own exporter before #1707 with only the `TIMESTAMP_MICROS` converted type, reads as the instant the Parquet spec defines it to be. The files are written by `python3` with pyarrow, so no test imports pyarrow: each declines by the shell's skip name, then asks for the fixture. On a private database. + +### Every test + +| test | what it holds | +| --- | --- | +| `test_ptz_pyarrow_wrote_one_naive_and_one_adjusted_file` | premise: pyarrow wrote the naive file without isAdjustedToUTC and the other with it | +| `test_ptz_a_naive_file_into_timestamptz_is_its_session_wall_time` | a naive file read into timestamptz is its wall time in the session zone | +| `test_ptz_the_naive_read_equals_the_cast_from_timestamp` | and that is what PostgreSQL's cast from timestamp gives | +| `test_ptz_an_adjusted_file_into_timestamp_is_the_session_wall_time` | an adjusted file read into timestamp is the session-zone wall time of its instant | +| `test_ptz_the_adjusted_read_equals_the_cast_from_timestamptz` | and that is what PostgreSQL's cast from timestamptz gives | +| `test_ptz_a_naive_file_into_timestamp_keeps_its_wall_time` | control: a naive file read into timestamp keeps its wall time | +| `test_ptz_an_adjusted_file_into_timestamptz_keeps_its_instant` | control: an adjusted file read into timestamptz keeps its instant | +| `test_ptz_an_export_import_round_trip_keeps_both_types` | an export and import round trip keeps timestamp and timestamptz in a non-UTC session | +| `test_ptz_the_gap_wall_times_become_their_instants` | the gap file's wall times become, sorted, the instants 07:00Z, 07:00Z and 07:30Z | +| `test_ptz_a_gap_spanning_group_is_not_skipped` | a row group spanning a spring-forward gap is not skipped on its converted min and max | +| `test_ptz_the_legacy_fixture_carries_only_the_converted_type` | premise: the legacy fixture carries the TIMESTAMP_MICROS converted type and no logical type | +| `test_ptz_a_naive_file_in_tokyo_is_its_wall_time_there` | a naive file read into timestamptz in Tokyo is its wall time there, as the cast gives | +| `test_ptz_the_tokyo_reading_differs_from_new_york` | and the Tokyo reading is a different instant from the New York one | +| `test_ptz_a_fall_back_wall_time_is_what_the_cast_gives` | a naive wall time in the fall-back hour read into timestamptz is what the cast gives | +| `test_ptz_a_legacy_file_into_timestamp_is_the_session_wall_time` | a legacy TIMESTAMP_MICROS file read into timestamp is the session-zone wall time of its instant | +| `test_ptz_a_legacy_file_into_timestamptz_keeps_its_instant` | a legacy TIMESTAMP_MICROS file read into timestamptz keeps its instant | diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index c2ef5cf38..5e30d3814 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -1210,4 +1210,6 @@ guard_tests 839 # 54 tests, one per check, by collection over the cluster files. # cluster_tests 4225 -> 4226 for #1753's arm 5 in test_parallel_flush_optin.py (folded into #1746): # one structural test, by collection over the cluster files. -cluster_tests 4231 +# cluster_tests 4231 -> 4249 for #1706: test_parquet_timestamp_zone.py, the twin of parquet_timestamp_zone.sh +# (16 tests, one per check), and two mixed-kind hour() arms in test_iceberg_fdw.py, by collection. +cluster_tests 4249 diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index 216ab6438..834787edf 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -207,6 +207,7 @@ "cancel_decode", "bench_guards", "catalog_delete_index", + "parquet_timestamp_zone", "catalog_filter_reads", "catalog_plan_index", "catalog_scan_index", diff --git a/test/pytest/test_iceberg_fdw.py b/test/pytest/test_iceberg_fdw.py index f87043d7e..ad7dd90ea 100644 --- a/test/pytest/test_iceberg_fdw.py +++ b/test/pytest/test_iceberg_fdw.py @@ -58,6 +58,9 @@ "mz": ("warehouse_temporal_xt", "bymonth_tz", "id bigint, tt timestamptz"), "dz": ("warehouse_temporal_xt", "byday_tz", "id bigint, tt timestamptz"), "hz": ("warehouse_temporal_xt", "byhour_tz", "id bigint, tt timestamptz"), + # the same two hour() tables declared as the OTHER timestamp type (#1706) + "hw": ("warehouse_temporal", "byhour", "id bigint, ts timestamptz"), + "hn": ("warehouse_temporal_xt", "byhour_tz", "id bigint, tt timestamp"), } @@ -647,6 +650,51 @@ def test_a_year_on_date_equality_prunes_to_one_file(ice, expect): "year()-on-date equality returns id 2") +# ------------------------------- a temporal transform read into the OTHER timestamp type +# +# The reader converts a naive (timestamp) source read into timestamptz, and a +# timestamptz source read into timestamp, as PostgreSQL's casts do in the session zone +# (#1706). The writer computed each partition cell in the source's own kind, so a cell +# does not bound the converted rows and the FDW must not prune on it. In New York, +# byhour's naive 2021-03-01 00:00, 01:00, 05:00 are 05:00Z, 06:00Z and 10:00Z; byhour_tz's +# 00:00Z, 01:00Z, 05:00Z are the wall times 2021-02-28 19:00, 20:00 and 2021-03-01 00:00. +# Measured before the guard: the FDW returned no rows where iceberg_scan returned two. + +def _in_new_york(conn, read): + """-> read(), with the shared connection's zone set to New York for that read only.""" + with conn.cursor() as cur: + cur.execute("SET TimeZone = 'America/New_York'") + try: + return read() + finally: + with conn.cursor() as cur: + cur.execute("RESET TimeZone") + + +def test_an_hour_partition_over_a_naive_source_read_as_timestamptz_keeps_its_rows(ice, expect): + conn, meta = ice + if not _need(expect, meta, "hw"): + return + where = "ts >= TIMESTAMPTZ '2021-03-01 06:00:00+00'" + got, want = _in_new_york(conn, lambda: ( + _ids(conn, "hw", where), + _scan_ids(conn, meta["hw"], "id bigint, ts timestamptz", where))) + expect.text(got, want, + "hour() on a naive source read as timestamptz in New York matches iceberg_scan") + + +def test_an_hour_partition_over_a_timestamptz_source_read_as_timestamp_keeps_its_rows(ice, expect): + conn, meta = ice + if not _need(expect, meta, "hn"): + return + where = "tt <= TIMESTAMP '2021-02-28 20:00:00'" + got, want = _in_new_york(conn, lambda: ( + _ids(conn, "hn", where), + _scan_ids(conn, meta["hn"], "id bigint, tt timestamp", where))) + expect.text(got, want, + "hour() on a timestamptz source read as timestamp in New York matches iceberg_scan") + + # ------------------------------------------------------------------- option validation def test_an_unknown_table_option_is_refused(ice, expect): diff --git a/test/pytest/test_parquet_timestamp_zone.py b/test/pytest/test_parquet_timestamp_zone.py new file mode 100644 index 000000000..6d4035185 --- /dev/null +++ b/test/pytest/test_parquet_timestamp_zone.py @@ -0,0 +1,261 @@ +"""A Parquet timestamp read into the other PostgreSQL timestamp type follows PostgreSQL's +cast semantics (#1706 item 2). The pytest twin of `test/parquet_timestamp_zone.sh`, every +check by the shell's name. + +Parquet marks a timestamp column isAdjustedToUTC: true for an instant, false for a wall +time. In a New York session, a naive file read into timestamptz must be its wall time in +the session zone, and an adjusted file read into timestamp the session-zone wall time of +its instant, each equal to PostgreSQL's own cast; the same-kind reads are the controls. An +export and import round trip keeps both types (it needs #1707's exporter). A row group that +spans the 2024-03-10 spring-forward gap must not be skipped on its converted min and max, +which need not bound its converted rows. The naive file read again in Tokyo must be the cast +in that zone, a different instant from New York's; the repeated hour of New York's 2024-11-03 +fall-back must resolve as the cast resolves it; and a file carrying only the legacy +TIMESTAMP_MICROS converted type (pgcolumnar's own export before #1707, checked in as +`test/fixtures/parquet/legacy_timestamp_micros.parquet`) is read as the instant the +Parquet spec defines it to be. + +The files are written by `python3` with pyarrow, as the shell writes them, so no test here +imports pyarrow: each declines by the shell's skip name first, and only then asks for the +module fixture. ON A PRIVATE DATABASE, every session at TimeZone America/New_York. +""" +import pathlib +import shutil +import subprocess +import sys + +import psycopg +import pytest + +from pgc_vacuity import query_error + +DB = "pgc_parquet_timestamp_zone" +ZONE = "-c TimeZone=America/New_York" +TOKYO = "-c TimeZone=Asia/Tokyo" +LEGACY = (pathlib.Path(__file__).resolve().parents[1] / "fixtures" / "parquet" + / "legacy_timestamp_micros.parquet") + +WRITER = r''' +import sys, datetime as dt, pyarrow as pa, pyarrow.parquet as pq +w = sys.argv[1] +noon = dt.datetime(2024, 6, 1, 12, 0) +pq.write_table(pa.table({"t": pa.array([noon], pa.timestamp("us"))}), f"{w}/naive.parquet") +pq.write_table(pa.table({"t": pa.array([noon], pa.timestamp("us", tz="UTC"))}), f"{w}/adjusted.parquet") +gap = [dt.datetime(2024, 3, 10, 2, 0), dt.datetime(2024, 3, 10, 2, 30), dt.datetime(2024, 3, 10, 3, 0)] +pq.write_table(pa.table({"t": pa.array(gap, pa.timestamp("us"))}), f"{w}/gap.parquet", row_group_size=3) +fb = [dt.datetime(2024, 11, 3, 1, 30)] +pq.write_table(pa.table({"t": pa.array(fb, pa.timestamp("us"))}), f"{w}/fallback.parquet") +a = lambda f: pq.ParquetFile(f).schema_arrow.field("t").type.tz +print(a(f"{w}/naive.parquet"), a(f"{w}/adjusted.parquet")) +''' + +# pyarrow derives a logical type from a converted type and says so in this flag +LEGACY_PREMISE = r''' +import sys, pyarrow.parquet as pq +c = pq.ParquetFile(sys.argv[1]).metadata.schema.column(0) +print(c.converted_type, "is_from_converted_type=true" in str(c.logical_type)) +''' + + +def _failed(exc): + return query_error(str(exc).splitlines()[0] if str(exc) else type(exc).__name__) + + +def _value(dsn, sql, zone=ZONE): + """-> the first column of the first row as text, in a New York session unless + `zone` names another, or query_error() carrying the failure.""" + try: + with psycopg.connect(dsn, autocommit=True, options=zone) as conn: + row = conn.execute(sql).fetchone() + except psycopg.Error as exc: + return _failed(exc) + return "" if row is None or row[0] is None else str(row[0]) + + +def _run(dsn, sql): + try: + with psycopg.connect(dsn, autocommit=True, options=ZONE) as conn: + conn.execute(sql) + except psycopg.Error as exc: + return _failed(exc) + return None + + +@pytest.fixture(scope="module") +def ptz(pgc_cluster, server_dir): + with psycopg.connect(pgc_cluster.dsn(), autocommit=True) as a: + a.execute(f"DROP DATABASE IF EXISTS {DB} WITH (FORCE)") + a.execute(f"CREATE DATABASE {DB}") + try: + dsn = pgc_cluster.dsn(dbname=DB) + w = server_dir + wrote = subprocess.run([sys.executable, "-c", WRITER, str(w)], capture_output=True, text=True) + shutil.copyfile(LEGACY, w / "legacy.parquet") + for f in ("naive", "adjusted", "gap", "fallback", "legacy"): + if (w / f"{f}.parquet").exists(): + (w / f"{f}.parquet").chmod(0o644) + r = {"tz of the files": wrote.stdout.strip() if wrote.returncode == 0 + else query_error(f"the writer failed: {wrote.stderr.strip().splitlines()[-1:]}")} + legacy = subprocess.run([sys.executable, "-c", LEGACY_PREMISE, str(w / "legacy.parquet")], + capture_output=True, text=True) + r["legacy premise"] = (legacy.stdout.strip() if legacy.returncode == 0 else + query_error(f"the reader failed: {legacy.stderr.strip().splitlines()[-1:]}")) + _run(dsn, "CREATE EXTENSION IF NOT EXISTS pgcolumnar") + _run(dsn, "CREATE SERVER pq FOREIGN DATA WRAPPER pgcolumnar_parquet") + for t, typ in (("n_ts", "timestamp"), ("n_tz", "timestamptz"), + ("a_ts", "timestamp"), ("a_tz", "timestamptz")): + _run(dsn, f"CREATE TABLE {t} (t {typ}) USING pgcolumnar") + src = "naive" if t.startswith("n_") else "adjusted" + _run(dsn, f"SELECT pgcolumnar.import_parquet('{t}', '{w}/{src}.parquet')") + for t in ("n_ts", "n_tz", "a_ts", "a_tz"): + r[t] = _value(dsn, f"SELECT t::text FROM {t}") + r["n_tz cast"] = _value(dsn, "SELECT ((SELECT t FROM n_tz) = " + "'2024-06-01 12:00'::timestamp::timestamptz)::text") + r["a_ts cast"] = _value(dsn, "SELECT ((SELECT t FROM a_ts) = " + "'2024-06-01 12:00+00'::timestamptz::timestamp)::text") + _run(dsn, "CREATE TABLE rt_src (ts timestamp, tz timestamptz) USING pgcolumnar") + _run(dsn, "INSERT INTO rt_src VALUES ('2024-06-01 12:00', '2024-06-01 12:00')") + _run(dsn, "CREATE TABLE rt_dst (ts timestamp, tz timestamptz) USING pgcolumnar") + _run(dsn, f"SELECT pgcolumnar.export_parquet('rt_src', '{w}/rt.parquet')") + _run(dsn, f"SELECT pgcolumnar.import_parquet('rt_dst', '{w}/rt.parquet')") + r["round trip"] = _value(dsn, "SELECT ts::text || ' | ' || tz::text FROM rt_dst") + _run(dsn, f"CREATE FOREIGN TABLE f_gap (t timestamptz) SERVER pq OPTIONS (path '{w}/gap.parquet')") + r["gap instants"] = _value(dsn, "SELECT string_agg(to_char(t AT TIME ZONE 'UTC', 'HH24:MI'), ',' " + "ORDER BY t) FROM f_gap") + r["gap count"] = _value(dsn, "SELECT count(*)::text FROM f_gap WHERE t > '2024-03-10 07:15+00'") + _run(dsn, f"CREATE FOREIGN TABLE f_naive (t timestamptz) SERVER pq OPTIONS (path '{w}/naive.parquet')") + r["tokyo"] = _value(dsn, "SELECT t::text || ' ' || (t = '2024-06-01 12:00'::timestamp::timestamptz)::text " + "FROM f_naive", zone=TOKYO) + r["tokyo differs"] = _value(dsn, "SELECT ((SELECT t FROM f_naive) <> (SELECT t FROM n_tz))::text", + zone=TOKYO) + _run(dsn, f"CREATE FOREIGN TABLE f_fb (t timestamptz) SERVER pq OPTIONS (path '{w}/fallback.parquet')") + r["fall-back"] = _value(dsn, "SELECT t::text || ' ' || (t = '2024-11-03 01:30'::timestamp::timestamptz)::text " + "FROM f_fb") + _run(dsn, f"CREATE FOREIGN TABLE f_leg_ts (t timestamp) SERVER pq OPTIONS (path '{w}/legacy.parquet')") + _run(dsn, f"CREATE FOREIGN TABLE f_leg_tz (t timestamptz) SERVER pq OPTIONS (path '{w}/legacy.parquet')") + r["legacy ts"] = _value(dsn, "SELECT t::text || ' ' || (t = '2024-06-01 12:00+00'::timestamptz::timestamp)::text " + "FROM f_leg_ts") + r["legacy tz"] = _value(dsn, "SELECT t::text FROM f_leg_tz") + yield r + finally: + with psycopg.connect(pgc_cluster.dsn(), autocommit=True) as a: + a.execute(f"DROP DATABASE IF EXISTS {DB} WITH (FORCE)") + + +def _readings(request, expect): + """Decline by the shell's skip name first; only then build the fixture.""" + if expect.require_module("pyarrow.parquet", + name="pyarrow not available; Parquet timestamp zone suite needs it") is None: + return None + return request.getfixturevalue("ptz") + + +def test_ptz_pyarrow_wrote_one_naive_and_one_adjusted_file(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["tz of the files"], "None UTC", + "premise: pyarrow wrote the naive file without isAdjustedToUTC and the other with it") + + +def test_ptz_a_naive_file_into_timestamptz_is_its_session_wall_time(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["n_tz"], "2024-06-01 12:00:00-04", + "a naive file read into timestamptz is its wall time in the session zone") + + +def test_ptz_the_naive_read_equals_the_cast_from_timestamp(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["n_tz cast"], "true", "and that is what PostgreSQL's cast from timestamp gives") + + +def test_ptz_an_adjusted_file_into_timestamp_is_the_session_wall_time(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["a_ts"], "2024-06-01 08:00:00", + "an adjusted file read into timestamp is the session-zone wall time of its instant") + + +def test_ptz_the_adjusted_read_equals_the_cast_from_timestamptz(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["a_ts cast"], "true", "and that is what PostgreSQL's cast from timestamptz gives") + + +def test_ptz_a_naive_file_into_timestamp_keeps_its_wall_time(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["n_ts"], "2024-06-01 12:00:00", + "control: a naive file read into timestamp keeps its wall time") + + +def test_ptz_an_adjusted_file_into_timestamptz_keeps_its_instant(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["a_tz"], "2024-06-01 08:00:00-04", + "control: an adjusted file read into timestamptz keeps its instant") + + +def test_ptz_an_export_import_round_trip_keeps_both_types(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["round trip"], "2024-06-01 12:00:00 | 2024-06-01 12:00:00-04", + "an export and import round trip keeps timestamp and timestamptz in a non-UTC session") + + +def test_ptz_the_gap_wall_times_become_their_instants(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["gap instants"], "07:00,07:00,07:30", + "the gap file's wall times 02:00, 02:30 and 03:00 become, sorted, the instants " + "07:00Z, 07:00Z and 07:30Z") + + +def test_ptz_a_gap_spanning_group_is_not_skipped(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["gap count"], "1", + "a row group spanning a spring-forward gap is not skipped on its converted min and max") + + +def test_ptz_the_legacy_fixture_carries_only_the_converted_type(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["legacy premise"], "TIMESTAMP_MICROS True", + "premise: the legacy fixture carries the TIMESTAMP_MICROS converted type and no logical type") + + +def test_ptz_a_naive_file_in_tokyo_is_its_wall_time_there(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["tokyo"], "2024-06-01 12:00:00+09 true", + "a naive file read into timestamptz in Tokyo is its wall time there, as the cast gives") + + +def test_ptz_the_tokyo_reading_differs_from_new_york(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["tokyo differs"], "true", + "and the Tokyo reading is a different instant from the New York one") + + +def test_ptz_a_fall_back_wall_time_is_what_the_cast_gives(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["fall-back"], "2024-11-03 01:30:00-05 true", + "a naive wall time in the fall-back hour read into timestamptz is what the cast gives") + + +def test_ptz_a_legacy_file_into_timestamp_is_the_session_wall_time(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["legacy ts"], "2024-06-01 08:00:00 true", + "a legacy TIMESTAMP_MICROS file read into timestamp is the session-zone wall time of its instant") + + +def test_ptz_a_legacy_file_into_timestamptz_keeps_its_instant(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["legacy tz"], "2024-06-01 08:00:00-04", + "a legacy TIMESTAMP_MICROS file read into timestamptz keeps its instant") diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index c3db03fd1..5d7c24419 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -261,6 +261,7 @@ SUITES=( parquet_level_width parquet_nested parquet_nested_import + parquet_timestamp_zone partition_lifecycle partition_rows partman_columnar From a4ea17561566950403c9b80847b7332a14f2bab6 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 22 Oct 2026 14:43:45 +0000 Subject: [PATCH 2/7] fix: Parquet row-group skipping uses a constant of another type in the operator family (#1712) The foreign table's group-exclusion check refused any constant whose type was not the column's. A bare integer literal is int4, so the most ordinary filter on a bigint column, `WHERE id > 150000`, read every row group; only `150000::int8` skipped. Answers were correct; reads were not saved. The check now compares the statistics with the constant through the operator family's comparison function for (column type, constant type), as the native zone-map path already did (OffgridwithJD's measurement on #1712). A family with no such function means no skip. Two details the old code would have got wrong for a cross-type constant: the NaN check read the constant as the column's float type (it now reads it by its own type), and equality called the comparison with the constant first (the column's value is now always the first argument). New suite test/parquet_skip_cross_type.sh and its twin, 9 checks, over a 200,000-row file in 4 row groups: int4 constants on an int8 column (>, =, and on the left), a negative float4 constant on a float8 column, the answers against the same-type query, and a float4 NaN that must still find the NaN row its group's statistics leave out. parquet_export_stats measured the old refusal in six arms ("a cross-type constant skips nothing", and five like it). Those assertions are now wrong by design: each is renamed for what it asserts, the skip its same-type twin gets, keeps its ledger row, and is re-dated by the pre-#1712 reader, which fails exactly those six in both harnesses. Three soundness arms are added for the changed predicates that had none ([i8 < 5::int], [i8 < 5], [f4 < 0.4]); every oracle arm passes. docs/limitations.md states the new rule. Measured on PG17: - Red: the pre-#1712 reader fails the four cross-type skip arms of the new suite, in shell and twin. - Green: 9/9 shell and twin on each of PG15 to PG19; parquet_export_stats 165/165 shell and twin on each of PG15 to PG19. - Mutants, the same names in shell and twin: the comparison called with the constant first (4, the same-type control among them); the column type's own function used for a constant of another type (the negative float4 arm); the NaN constant read as the column's type (the NaN arm). - 47 parquet, iceberg and arrow suites on PG17: only parquet_export_stats's six refusal arms failed, before they were rewritten; 1044 checks. - docs_style passes. Guards 476 passed. cluster_tests 2763 -> 2775 (collected 2775). Ledger 6180 + 12 = 6192 rows, 3886 -> 3892 never, 2294 -> 2300 dated; the gate passes, ceiling 102. - Rebased onto #1736 at a93f14bf, the registry re-derived there: cluster_tests 3240 -> 3252 (collected 3252). Ledger 6951 + 12 = 6963 rows, 4140 -> 4146 never, 2811 -> 2817 dated. The gate is run on the commit after this one. Stacked on #1736 (and #1707), which touch the same function. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 7 + docs/limitations.md | 8 +- src/columnar_parquet_reader.c | 58 +++++-- test/check_ledger.tsv | 24 ++- test/check_ledger_budget.txt | 27 ++- test/parquet_export_stats.sh | 48 +++--- test/parquet_skip_cross_type.sh | 77 +++++++++ test/pytest/TESTS.md | 33 +++- test/pytest/expected_tests.txt | 6 +- test/pytest/test_compare_to_bash.py | 1 + test/pytest/test_parquet_export_stats.py | 40 +++-- test/pytest/test_parquet_skip_cross_type.py | 175 ++++++++++++++++++++ test/run_all_versions.sh | 1 + 13 files changed, 437 insertions(+), 68 deletions(-) create mode 100755 test/parquet_skip_cross_type.sh create mode 100644 test/pytest/test_parquet_skip_cross_type.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 1db70a33d..20b087f0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -444,6 +444,13 @@ true until the next version shipped. ended the cleanup early: the segments stayed pinned until the server restarted, and the second cancel was reported instead of the first. The cleanup now finishes whatever arrives, in the time a single cancel already took (#1745). +- **Parquet row-group skipping now uses a constant of another integer or float + type.** A bare integer literal is `int4`, so `WHERE id > 150000` on a `bigint` + column of a Parquet foreign table read every row group; only `150000::int8` + skipped. The check now compares through the operator family's cross-type + comparison, as the native columnar scan already did. Answers were always correct; + this only reads less (#1712). `test/parquet_skip_cross_type.sh` and + `test_parquet_skip_cross_type.py`. - **`export_parquet` no longer marks a `timestamp` column as a UTC instant.** Both `timestamp` and `timestamptz` were written as the converted type `TIMESTAMP_MICROS`, which the Parquet spec defines as adjusted to UTC. Every other reader (pyarrow, diff --git a/docs/limitations.md b/docs/limitations.md index d64ba6fe0..2b657adc0 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -1091,9 +1091,11 @@ correct rows; these are the conditions under which it can skip at all: bytea, uuid, and boolean columns are filtered but never skipped, whatever their statistics. A `numeric` column follows its storage: one written as an INT32 or INT64 DECIMAL does skip, one written as a byte-array DECIMAL does not. -- The constant's type must match the column's type exactly. A cross-type - comparison such as `ts >= DATE '2026-01-01'` against a `timestamp` column, or - `bigint_col > 5::int`, does not skip. +- The constant may be of another type in the operator's family. So + `bigint_col > 5` skips, though `5` is an `int4`. So do `real_col < 0.4` and + `ts >= DATE '2026-01-01'` on a `timestamp` column. A comparison that casts the + column instead does not skip. An example is `bigint_col < 5.0`, where `5.0` is a + `numeric`. - The row group's statistics must carry both a minimum and a maximum, and the interval must not be inverted. Two cases decode to an interval that the reader does not trust for skipping. The first is an unsigned Parquet column that diff --git a/src/columnar_parquet_reader.c b/src/columnar_parquet_reader.c index 16d0a5991..6a6f1c363 100644 --- a/src/columnar_parquet_reader.c +++ b/src/columnar_parquet_reader.c @@ -34,6 +34,7 @@ #include "fmgr.h" #include "funcapi.h" +#include "access/nbtree.h" #include "access/htup_details.h" #include "common/int.h" #include "access/relation.h" @@ -4534,9 +4535,11 @@ pqfdw_top_for_attno(ImpTop *tops, int ntops, int attno0) */ static bool pqfdw_clause_excludes_group(ImpLeaf *leaf, PqChunk *ch, int strategy, - bool varLeft, Const *con) + bool varLeft, Const *con, Oid opfamily) { TypeCacheEntry *tce; + FmgrInfo cross; + FmgrInfo *vsConst; const uint8 *p; bool ok; Datum minD; @@ -4549,7 +4552,7 @@ pqfdw_clause_excludes_group(ImpLeaf *leaf, PqChunk *ch, int strategy, if (phys != PQ_INT32 && phys != PQ_INT64 && phys != PQ_FLOAT && phys != PQ_DOUBLE) return false; - if (con->consttype != leaf->plan.typid || con->constisnull) + if (con->constisnull) return false; /* * A wall time is not monotonic in its instant across a spring-forward gap, so @@ -4562,6 +4565,27 @@ pqfdw_clause_excludes_group(ImpLeaf *leaf, PqChunk *ch, int strategy, if (!OidIsValid(tce->cmp_proc_finfo.fn_oid)) return false; + /* + * The statistics are compared with the constant through the operator family's + * comparison function for (column type, constant type), as the native zone-map + * path does (#1712). A bare integer literal is int4, so requiring the column's + * own type left `WHERE id > 150000` on a bigint column skipping nothing. A + * family with no such function, or no family, means no skip. + */ + if (con->consttype == leaf->plan.typid) + vsConst = &tce->cmp_proc_finfo; + else + { + Oid proc = OidIsValid(opfamily) + ? get_opfamily_proc(opfamily, leaf->plan.typid, con->consttype, BTORDER_PROC) + : InvalidOid; + + if (!OidIsValid(proc)) + return false; + fmgr_info(proc, &cross); + vsConst = ✗ + } + p = ch->stat_min; minD = plain_value_to_datum(&leaf->plan, &p, p + ch->stat_min_len, false, &ok); if (!ok) @@ -4628,9 +4652,12 @@ pqfdw_clause_excludes_group(ImpLeaf *leaf, PqChunk *ch, int strategy, ? (float8) DatumGetFloat4(minD) : DatumGetFloat8(minD); float8 hi = (phys == PQ_FLOAT) ? (float8) DatumGetFloat4(maxD) : DatumGetFloat8(maxD); - float8 c = (phys == PQ_FLOAT) + /* the constant is read by ITS type, which may not be the column's */ + float8 c = (con->consttype == FLOAT4OID) ? (float8) DatumGetFloat4(con->constvalue) - : DatumGetFloat8(con->constvalue); + : (con->consttype == FLOAT8OID) + ? DatumGetFloat8(con->constvalue) + : 0.0; if (isnan(lo) || isnan(hi)) return false; @@ -4641,22 +4668,29 @@ pqfdw_clause_excludes_group(ImpLeaf *leaf, PqChunk *ch, int strategy, return false; } + /* + * The column's value is always the FIRST argument: a cross-type comparison + * function is defined for (column type, constant type) only. + */ +#define PQ_VS(a) \ + DatumGetInt32(FunctionCall2Coll(vsConst, InvalidOid, (a), con->constvalue)) + switch (strategy) { case BTLessStrategyNumber: /* col < const: skip if min >= const */ - return PQ_CMP(minD, con->constvalue) >= 0; + return PQ_VS(minD) >= 0; case BTLessEqualStrategyNumber: /* col <= const: skip if min > const */ - return PQ_CMP(minD, con->constvalue) > 0; + return PQ_VS(minD) > 0; case BTEqualStrategyNumber: /* col = const: skip if const outside [min,max] */ - return PQ_CMP(con->constvalue, minD) < 0 || - PQ_CMP(con->constvalue, maxD) > 0; + return PQ_VS(minD) > 0 || PQ_VS(maxD) < 0; case BTGreaterEqualStrategyNumber: /* col >= const: skip if max < const */ - return PQ_CMP(maxD, con->constvalue) < 0; + return PQ_VS(maxD) < 0; case BTGreaterStrategyNumber: /* col > const: skip if max <= const */ - return PQ_CMP(maxD, con->constvalue) <= 0; + return PQ_VS(maxD) <= 0; default: return false; } +#undef PQ_VS #undef PQ_CMP } @@ -4701,6 +4735,7 @@ pqfdw_compute_skip(ForeignScanState *node, PqFile *pf, List *interp; ListCell *ic; int strategy = 0; + Oid opfamily = InvalidOid; if (!IsA(op, OpExpr) || list_length(op->args) != 2) continue; @@ -4741,6 +4776,7 @@ pqfdw_compute_skip(ForeignScanState *node, PqFile *pf, if (s >= BTLessStrategyNumber && s <= BTGreaterStrategyNumber) { strategy = s; + opfamily = o->opfamily_id; break; } } @@ -4749,7 +4785,7 @@ pqfdw_compute_skip(ForeignScanState *node, PqFile *pf, if (pqfdw_clause_excludes_group(&leaves[top->firstLeaf], &pf->rgs[rg].chunks[top->firstLeaf], - strategy, varLeft, con)) + strategy, varLeft, con, opfamily)) { skipGroup[rg] = true; break; diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index ace68abc1..b1e1daace 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -5623,7 +5623,7 @@ parquet_count_bounds parquet_count_bounds the num_rows rejection names num_rows parquet_count_bounds parquet_count_bounds the num_values rejection names num_values in its detail 15;16;17;18;19 never - parquet_count_bounds parquet_count_bounds the server is still up after the honest import 15;16;17;18;19 never - parquet_export_stats parquet_export_stats a bigint predicate skips 15;16;17;18;19 2026-10-21 columnar_parquet.c: pqstats_reset leaves hasBounds set, so each later group's bounds start from the reset 0;columnar_parquet.c: write_statistics writes no min_value or max_value for any type;columnar_parquet_reader.c: col < const skips a group on max >= const instead of min >= const -parquet_export_stats parquet_export_stats a cross-type constant skips nothing, on the identical predicate 15;16;17;18;19 2026-10-21 columnar_parquet_reader.c: row-group skipping no longer refuses a constant of another type +parquet_export_stats parquet_export_stats a cross-type constant skips as the matching one does, on the identical predicate 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the exact-type refusal before #1712 restored, so a constant of another type skips nothing parquet_export_stats parquet_export_stats a date predicate skips 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds;columnar_parquet.c: pqstats_reset leaves hasBounds set, so each later group's bounds start from the reset 0;columnar_parquet.c: write_statistics writes no min_value or max_value for any type;columnar_parquet_reader.c: col < const skips a group on max >= const instead of min >= const parquet_export_stats parquet_export_stats a decimal-point literal casts the column, not the constant 15;16;17;18;19 never - parquet_export_stats parquet_export_stats a float >= predicate above every bound still skips nothing 15;16;17;18;19 2026-10-21 columnar_parquet_reader.c: row-group skipping no longer refuses > and >= on a float column @@ -5649,8 +5649,8 @@ parquet_export_stats parquet_export_stats control: the parser reports the deprec parquet_export_stats parquet_export_stats control: the quoted form casts the constant to the column's type 15;16;17;18;19 never - parquet_export_stats parquet_export_stats control: the same column skips when the constant matches its type 15;16;17;18;19 2026-10-21 columnar_parquet.c: pqstats_reset leaves hasBounds set, so each later group's bounds start from the reset 0;columnar_parquet.c: write_statistics writes no min_value or max_value for any type parquet_export_stats parquet_export_stats control: the unadorned form resolves to the column's type instead 15;16;17;18;19 never - +parquet_export_stats parquet_export_stats digits too wide for an integer are a bigint, and an int4 column skips as well 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the exact-type refusal before #1712 restored, so a constant of another type skips nothing parquet_export_stats parquet_export_stats digits too wide for an integer are a bigint, and skip 15;16;17;18;19 2026-10-21 columnar_parquet.c: pqstats_reset leaves hasBounds set, so each later group's bounds start from the reset 0;columnar_parquet.c: write_statistics writes no min_value or max_value for any type;columnar_parquet_reader.c: col < const skips a group on max >= const instead of min >= const -parquet_export_stats parquet_export_stats digits too wide for an integer are a bigint, so an int4 column refuses 15;16;17;18;19 2026-10-21 columnar_parquet_reader.c: row-group skipping no longer refuses a constant of another type parquet_export_stats parquet_export_stats each bucket holds its own contiguous id range 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds parquet_export_stats parquet_export_stats every ColumnOrder is TYPE_ORDER 15;16;17;18;19 2026-10-21 columnar_parquet.c: FileMetaData.column_orders not written parquet_export_stats parquet_export_stats every bound is its physical width 15;16;17;18;19 2026-10-21 columnar_parquet.c: write_statistics writes no min_value or max_value for any type @@ -5759,6 +5759,7 @@ parquet_export_stats parquet_export_stats the FDW result equals the oracle for [ parquet_export_stats parquet_export_stats the FDW result equals the oracle for [d < TIMESTAMP '2000-01-05 00:00:00'] 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds parquet_export_stats parquet_export_stats the FDW result equals the oracle for [d > DATE '2060-01-01'] 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds parquet_export_stats parquet_export_stats the FDW result equals the oracle for [f4 < '0.4'] 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds +parquet_export_stats parquet_export_stats the FDW result equals the oracle for [f4 < 0.4] 15;16;17;18;19 never - parquet_export_stats parquet_export_stats the FDW result equals the oracle for [f4 < 1.0::real] 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds;columnar_parquet_reader.c: col < const skips a group on max >= const instead of min >= const parquet_export_stats parquet_export_stats the FDW result equals the oracle for [f4 >= 1.0::real] 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds parquet_export_stats parquet_export_stats the FDW result equals the oracle for [i2 < '500'] 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds;columnar_parquet_reader.c: col < const skips a group on max >= const instead of min >= const @@ -5767,6 +5768,8 @@ parquet_export_stats parquet_export_stats the FDW result equals the oracle for [ parquet_export_stats parquet_export_stats the FDW result equals the oracle for [i8 < '5'] 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds parquet_export_stats parquet_export_stats the FDW result equals the oracle for [i8 < 1000000000::bigint] 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds;columnar_parquet_reader.c: col < const skips a group on max >= const instead of min >= const parquet_export_stats parquet_export_stats the FDW result equals the oracle for [i8 < 5000000000] 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds;columnar_parquet_reader.c: col < const skips a group on max >= const instead of min >= const +parquet_export_stats parquet_export_stats the FDW result equals the oracle for [i8 < 5::int] 15;16;17;18;19 never - +parquet_export_stats parquet_export_stats the FDW result equals the oracle for [i8 < 5] 15;16;17;18;19 never - parquet_export_stats parquet_export_stats the FDW result equals the oracle for [id < -1] 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds parquet_export_stats parquet_export_stats the FDW result equals the oracle for [id < -5000000000] 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds parquet_export_stats parquet_export_stats the FDW result equals the oracle for [id < 5000] 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds;columnar_parquet_reader.c: col < const skips a group on max >= const instead of min >= const @@ -5779,11 +5782,11 @@ parquet_export_stats parquet_export_stats the footer carries one ColumnOrder per parquet_export_stats parquet_export_stats the four groups report four distinct id minima 15;16;17;18;19 2026-10-21 columnar_parquet.c: pqstats_reset leaves hasBounds set, so each later group's bounds start from the reset 0;columnar_parquet.c: write_statistics writes no min_value or max_value for any type parquet_export_stats parquet_export_stats the oracle holds every exported row 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds parquet_export_stats parquet_export_stats the oracle's buckets are the file's row groups 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds -parquet_export_stats parquet_export_stats the same date adorned as a timestamp does not skip 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds +parquet_export_stats parquet_export_stats the same date adorned as a timestamp skips as well 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the exact-type refusal before #1712 restored, so a constant of another type skips nothing parquet_export_stats parquet_export_stats the same date unadorned is unknown, takes the column's type and skips 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds;columnar_parquet.c: pqstats_reset leaves hasBounds set, so each later group's bounds start from the reset 0;columnar_parquet.c: write_statistics writes no min_value or max_value for any type;columnar_parquet_reader.c: col < const skips a group on max >= const instead of min >= const -parquet_export_stats parquet_export_stats the same digits unquoted are an integer, and skip nothing 15;16;17;18;19 2026-10-21 columnar_parquet_reader.c: row-group skipping no longer refuses a constant of another type -parquet_export_stats parquet_export_stats the same digits unquoted are an integer, and skip nothing (int2) 15;16;17;18;19 2026-10-21 columnar_parquet_reader.c: row-group skipping no longer refuses a constant of another type -parquet_export_stats parquet_export_stats the same literal unquoted is double precision, and skips nothing 15;16;17;18;19 2026-10-21 columnar_parquet_reader.c: row-group skipping no longer refuses a constant of another type +parquet_export_stats parquet_export_stats the same digits unquoted are an integer, and skip as well 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the exact-type refusal before #1712 restored, so a constant of another type skips nothing +parquet_export_stats parquet_export_stats the same digits unquoted are an integer, and skip as well (int2) 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the exact-type refusal before #1712 restored, so a constant of another type skips nothing +parquet_export_stats parquet_export_stats the same literal unquoted is double precision, and skips as well 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the exact-type refusal before #1712 restored, so a constant of another type skips nothing parquet_level_width parquet_level_width a non-positive maximum returns before shifting at all 15;16;17;18;19 2026-10-21 columnar_parquet_format.h: pq_bits_for's non-positive guard is commented out parquet_level_width parquet_level_width and both callers reach the shared one 15;16;17;18;19 never - parquet_level_width parquet_level_width and no signed 1 << remains in it 15;16;17;18;19 2026-10-21 columnar_parquet_format.h: pq_bits_for shifts a signed 1 @@ -5804,6 +5807,15 @@ parquet_nested_import parquet_nested_import nested import round-trips (arrays + parquet_nested_import parquet_nested_import pyarrow import rows 15;16;17;18;19 never - parquet_nested_import parquet_nested_import pyarrow nested file matches oracle 15;16;17;18;19 2026-10-21 columnar_parquet_reader.c: an empty Parquet list is read back as a NULL array parquet_nested_import parquet_nested_import round-trips after delete 15;16;17;18;19 2026-10-21 columnar_parquet_reader.c: an empty Parquet list is read back as a NULL array +parquet_skip_cross_type parquet_skip_cross_type a constant on the left of the operator is compared the same way 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the comparison function is called with the constant first, not the column's value +parquet_skip_cross_type parquet_skip_cross_type a float4 NaN constant still finds the float8 column's NaN row 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the float NaN check reads the constant as the column's float type +parquet_skip_cross_type parquet_skip_cross_type a float4 constant below every row skips all 4 row groups of the float8 column 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the column type's own comparison function is used for a constant of another type +parquet_skip_cross_type parquet_skip_cross_type an int4 constant in an equality on the int8 column skips 3 of the 4 row groups 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the comparison function is called with the constant first, not the column's value +parquet_skip_cross_type parquet_skip_cross_type an int4 constant on the int8 column skips 3 of the 4 row groups 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the comparison function is called with the constant first, not the column's value +parquet_skip_cross_type parquet_skip_cross_type and returns the rows the float8 constant returns 15;16;17;18;19 never - +parquet_skip_cross_type parquet_skip_cross_type and returns the rows the int8 constant returns 15;16;17;18;19 never - +parquet_skip_cross_type parquet_skip_cross_type control: an int8 constant on the int8 column skips 3 of the 4 row groups 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the comparison function is called with the constant first, not the column's value +parquet_skip_cross_type parquet_skip_cross_type premise: the file holds 4 row groups (pyarrow's own count) 15;16;17;18;19 never - parquet_timestamp_zone parquet_timestamp_zone a legacy TIMESTAMP_MICROS file read into timestamp is the session-zone wall time of its instant 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: a column with only a legacy TIMESTAMP_* converted type reads as not UTC-adjusted parquet_timestamp_zone parquet_timestamp_zone a legacy TIMESTAMP_MICROS file read into timestamptz keeps its instant 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: a column with only a legacy TIMESTAMP_* converted type reads as not UTC-adjusted parquet_timestamp_zone parquet_timestamp_zone a naive file read into timestamptz in Tokyo is its wall time there, as the cast gives 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: a naive column read into timestamptz is converted in America/New_York, not the session zone diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index b304ad7de..51327c27c 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -7074,4 +7074,29 @@ suites_not_covered 27 # never 4535 + 3 = 4538 # dated 3445 + 13 = 3458 # 4538 + 3458 == 7996 -checks_never_observed_red 4538 +# +# #1712, parquet_skip_cross_type seeded, with its twin (jdatcmd): 4538 -> 4541. A new suite, +# so suites_not_covered does not move: it is registered and covered in the same change. It +# enters with 9 checks from five-major runs (all green on each of 15-19, shell and twin +# alike). 6 are dated by three mutations, each reddening the same names in both twins: the +# comparison function called with the constant first (4), the column type's own function +# used for a constant of another type (the negative float4 arm, 1), and the NaN check +# reading the constant as the column's type (1). The premise and the two answer controls +# arrive never. +# +# ledger rows 7996 + 9 = 8005 +# never 4538 + 3 = 4541 +# dated 3458 + 6 = 3464 +# 4541 + 3464 == 8005 +# +# #1712, parquet_export_stats follows the new rule (jdatcmd): 4541 -> 4544. Six arms measured +# the refusal of a cross-type constant (it skipped nothing); each is renamed for what it now +# asserts, the skip its same-type twin gets, and keeps its row's history. Three soundness +# arms are added for the predicates that had none, [i8 < 5::int], [i8 < 5] and [f4 < 0.4], +# and arrive never, like the other oracle arms. suites_not_covered does not move. +# +# ledger rows 8005 + 3 = 8008 +# never 4541 + 3 = 4544 +# dated 3464 + 0 = 3464 +# 4544 + 3464 == 8008 +checks_never_observed_red 4544 diff --git a/test/parquet_export_stats.sh b/test/parquet_export_stats.sh index e2e00ea68..0fa69141c 100755 --- a/test/parquet_export_stats.sh +++ b/test/parquet_export_stats.sh @@ -612,7 +612,11 @@ check_num "a date predicate skips" \ "$(skipped_for "WHERE d < DATE '2000-01-05'")" "3" check_num "a timestamp predicate skips" \ "$(skipped_for "WHERE ts < TIMESTAMP '2000-01-01 00:30:00'")" "3" -# Documented refusals, reachable from our own exported files for the first time. +# CROSS-TYPE CONSTANTS. These arms measured a documented refusal: a constant whose +# type was not the column's skipped nothing. Since #1712 the reader compares through +# the operator family's cross-type function, so each now skips exactly as its +# same-type twin beside it does, and the soundness arms below cover every one. +# The comments that follow describe how each pair was built. # # Each runs beside a predicate that differs in ONE respect and does skip. Without # that pair the checks are worthless: every group's max(i8) exceeds 5 and every @@ -621,8 +625,8 @@ check_num "a timestamp predicate skips" \ # not evidence of a refusal. check_num "control: the same column skips when the constant matches its type" \ "$(skipped_for "WHERE i8 < 5::bigint")" "$NGROUPS" -check_num "a cross-type constant skips nothing, on the identical predicate" \ - "$(skipped_for "WHERE i8 < 5::int")" "0" +check_num "a cross-type constant skips as the matching one does, on the identical predicate" \ + "$(skipped_for "WHERE i8 < 5::int")" "$NGROUPS" check_num "control: a float < predicate below every bound skips" \ "$(skipped_for "WHERE f4 < 0.4::real")" "$NGROUPS" # The reader refuses > and >= on a float column outright, because Parquet @@ -644,8 +648,8 @@ check_num "a float >= predicate above every bound still skips nothing" \ # constant cannot produce, which is what the pair is for. check_num "a quoted literal takes a bigint column's type and skips" \ "$(skipped_for "WHERE i8 < '5'")" "$NGROUPS" -check_num "the same digits unquoted are an integer, and skip nothing" \ - "$(skipped_for "WHERE i8 < 5")" "0" +check_num "the same digits unquoted are an integer, and skip as well" \ + "$(skipped_for "WHERE i8 < 5")" "$NGROUPS" check_num "digits too wide for an integer are a bigint, and skip" \ "$(skipped_for "WHERE i8 < 5000000000")" "3" # A decimal-point literal is a `numeric`, and PostgreSQL resolves @@ -665,12 +669,12 @@ check_num "control: the quoted form casts the constant to the column's type" \ "$(filter_has "WHERE i8 < '5'" '::bigint')" "1" check_num "a quoted literal takes a real column's type and skips" \ "$(skipped_for "WHERE f4 < '0.4'")" "$NGROUPS" -check_num "the same literal unquoted is double precision, and skips nothing" \ - "$(skipped_for "WHERE f4 < 0.4")" "0" +check_num "the same literal unquoted is double precision, and skips as well" \ + "$(skipped_for "WHERE f4 < 0.4")" "$NGROUPS" check_num "a quoted literal takes a smallint column's type and skips" \ "$(skipped_for "WHERE i2 < '500'")" "1" -check_num "the same digits unquoted are an integer, and skip nothing (int2)" \ - "$(skipped_for "WHERE i2 < 500")" "0" +check_num "the same digits unquoted are an integer, and skip as well (int2)" \ + "$(skipped_for "WHERE i2 < 500")" "1" # ADORNED is not the same as quoted. A literal that names its own type is that # type, not `unknown`, so it does not take the column's and the exact-type @@ -682,16 +686,12 @@ check_num "an adorned literal of the column's own type skips" \ "$(skipped_for "WHERE d < DATE '2000-01-05'")" "3" check_num "the same date unadorned is unknown, takes the column's type and skips" \ "$(skipped_for "WHERE d < '2000-01-05'")" "3" -check_num "the same date adorned as a timestamp does not skip" \ - "$(skipped_for "WHERE d < TIMESTAMP '2000-01-05 00:00:00'")" "0" -# That last arm is a documented-behaviour guard rather than evidence, and the -# distinction is worth stating rather than leaving for a reader to discover. -# No mutation in this suite's ledger reddens it: with the reader's -# exact-constant-type refusal deleted, the timestamp datum read as a date lands -# outside this fixture's range in the direction that prunes nothing, so the arm -# still reports 0 for a second reason. What IS falsifiable is the typing the -# sentence rests on, and the planner will say it: an adorned literal keeps its -# own type beside a bare Var, while the unadorned form resolves to the column's. +check_num "the same date adorned as a timestamp skips as well" \ + "$(skipped_for "WHERE d < TIMESTAMP '2000-01-05 00:00:00'")" "3" +# The typing the pair rests on is falsifiable, and the planner will say it: an +# adorned literal keeps its own type beside a bare Var, while the unadorned form +# resolves to the column's. Since #1712 both forms skip, through different +# comparison functions. check_num "an adorned literal keeps its own type, not the column's" \ "$(filter_has "WHERE d < TIMESTAMP '2000-01-05 00:00:00'" '::timestamp')" "1" check_num "control: the unadorned form resolves to the column's type instead" \ @@ -704,8 +704,8 @@ check_num "control: the unadorned form resolves to the column's type instead" \ # and the other prunes none. check_num "control: a digit string that fits an integer prunes every group" \ "$(skipped_for "WHERE id < -1")" "$NGROUPS" -check_num "digits too wide for an integer are a bigint, so an int4 column refuses" \ - "$(skipped_for "WHERE id < -5000000000")" "0" +check_num "digits too wide for an integer are a bigint, and an int4 column skips as well" \ + "$(skipped_for "WHERE id < -5000000000")" "$NGROUPS" # Soundness. A skipped group emits nothing, and the executor's recheck cannot # recover a row that never arrived, so every predicate above must return exactly @@ -755,6 +755,12 @@ fdw_vs_oracle "the FDW result equals the oracle for [id < -5000000000]" \ "id < -5000000000" fdw_vs_oracle "the FDW result equals the oracle for [alln IS NULL]" \ "alln IS NULL" +fdw_vs_oracle "the FDW result equals the oracle for [i8 < 5::int]" \ + "i8 < 5::int" +fdw_vs_oracle "the FDW result equals the oracle for [i8 < 5]" \ + "i8 < 5" +fdw_vs_oracle "the FDW result equals the oracle for [f4 < 0.4]" \ + "f4 < 0.4" # ---- the other direction: the instrument can report a bound that IS there --- # diff --git a/test/parquet_skip_cross_type.sh b/test/parquet_skip_cross_type.sh new file mode 100755 index 000000000..3f61c1ada --- /dev/null +++ b/test/parquet_skip_cross_type.sh @@ -0,0 +1,77 @@ +#!/usr/bin/env bash +# +# Parquet row-group skipping uses a constant of another type in the column's +# operator family (#1712). +# +# The foreign table's group-exclusion check refused any constant whose type was +# not the column's. A bare integer literal is int4, so the most ordinary filter +# on a bigint column, `WHERE id > 150000`, read every row group; only +# `150000::int8` skipped. The same held for a float4 constant on a float8 column. +# The native columnar path already compares through the operator family's +# cross-type comparison function, and this suite asks the same of Parquet. +# +# A file of 200,000 ascending rows in 4 row groups of 50,000. Each arm reads the +# skip count from EXPLAIN (ANALYZE) and, beside it, the answer, compared with the +# same query written with a constant of the column's own type. +# +# Usage: test/parquet_skip_cross_type.sh [PG_CONFIG] + +set -uo pipefail +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" + +if ! python3 -c 'import pyarrow.parquet' 2>/dev/null; then + pgc_skip pyarrow "pyarrow not available; Parquet cross-type skip suite needs it" +fi + +W="$PGC_WORKDIR" +NGROUPS="$(python3 - "$W" <<'PY' +import sys, pyarrow as pa, pyarrow.parquet as pq +w = sys.argv[1] +ids = list(range(200000)) +pq.write_table(pa.table({"id": pa.array(ids, pa.int64()), + "f": pa.array([float(i) for i in ids], pa.float64())}), + f"{w}/xt.parquet", row_group_size=50000) +# NaN sits in the second group; Parquet statistics leave it out of min and max +pq.write_table(pa.table({"f": pa.array([1.0, 2.0, 3.0, float("nan")], pa.float64())}), + f"{w}/xn.parquet", row_group_size=2) +print(pq.ParquetFile(f"{w}/xt.parquet").metadata.num_row_groups) +PY +)" +chmod 644 "$W/xt.parquet" "$W/xn.parquet" +psql_run "CREATE SERVER pq FOREIGN DATA WRAPPER pgcolumnar_parquet; + CREATE FOREIGN TABLE xt (id int8, f float8) SERVER pq OPTIONS (path '$W/xt.parquet'); + CREATE FOREIGN TABLE xn (f float8) SERVER pq OPTIONS (path '$W/xn.parquet');" + +# skipped QUERY -> the "Row Groups Skipped" count EXPLAIN (ANALYZE) reports, or the error +skipped() { + local out + out="$(q "EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF) $1" 2>&1)" + grep -oE 'Row Groups Skipped: [0-9]+' <<<"$out" | grep -oE '[0-9]+$' || echo "no reading: $(head -1 <<<"$out")" +} + +check_num "premise: the file holds 4 row groups (pyarrow's own count)" "$NGROUPS" "4" +check_num "control: an int8 constant on the int8 column skips 3 of the 4 row groups" \ + "$(skipped "SELECT count(*) FROM xt WHERE id > 150000::int8")" "3" +check_num "an int4 constant on the int8 column skips 3 of the 4 row groups" \ + "$(skipped "SELECT count(*) FROM xt WHERE id > 150000")" "3" +check_num "and returns the rows the int8 constant returns" \ + "$(q "SELECT count(*) FROM xt WHERE id > 150000;")" "$(q "SELECT count(*) FROM xt WHERE id > 150000::int8;")" +check_num "an int4 constant in an equality on the int8 column skips 3 of the 4 row groups" \ + "$(skipped "SELECT count(*) FROM xt WHERE id = 150001")" "3" +check_num "a constant on the left of the operator is compared the same way" \ + "$(skipped "SELECT count(*) FROM xt WHERE 150000 < id")" "3" +# `<` because the float rule never skips on `>` or `>=` (a NaN row satisfies them +# and sits outside min/max), and a NEGATIVE constant because a float4 misread as +# float8 is a tiny positive number: then group 0 (min 0.0) would not be skipped. +check_num "a float4 constant below every row skips all 4 row groups of the float8 column" \ + "$(skipped "SELECT count(*) FROM xt WHERE f < -1::float4")" "4" +check_num "and returns the rows the float8 constant returns" \ + "$(q "SELECT count(*) FROM xt WHERE f < 50000::float4;")" "$(q "SELECT count(*) FROM xt WHERE f < 50000::float8;")" + +# A NaN constant is read by its own type. Read as the column's float8, a float4 NaN +# is some other number, and the group holding the NaN row would be skipped. +check_num "a float4 NaN constant still finds the float8 column's NaN row" \ + "$(q "SELECT count(*) FROM xn WHERE f = 'NaN'::float4;")" "1" + +pgc_summary diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index d2a4db9c0..d17facb4a 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -389,6 +389,7 @@ behaviour, the source of that number is named. - [341. test_native_cancel.py: a scan's row-group load yields to statement_timeout inside the decode loop](#341-test_native_cancelpy-a-scans-row-group-load-yields-to-statement_timeout-inside-the-decode-loop) - [342. test_objstore_export_double_cancel.py: a second cancel during a remote export's cleanup keeps the export's own error](#342-test_objstore_export_double_cancelpy-a-second-cancel-during-a-remote-exports-cleanup-keeps-the-exports-own-error) - [343. test_parquet_timestamp_zone.py: a Parquet timestamp read into the other timestamp type follows PostgreSQL's casts](#343-test_parquet_timestamp_zonepy-a-parquet-timestamp-read-into-the-other-timestamp-type-follows-postgresqls-casts) +- [344. test_parquet_skip_cross_type.py: Parquet row-group skipping uses a constant of another type in the operator family](#344-test_parquet_skip_cross_typepy-parquet-row-group-skipping-uses-a-constant-of-another-type-in-the-operator-family) ## 1. How to read a test in here @@ -10372,26 +10373,26 @@ The bound and null-count oracles read the heap built from the generator, not the | `test_export_stats_date_predicate_skips` | a date predicate skips | | `test_export_stats_timestamp_predicate_skips` | a timestamp predicate skips | | `test_export_stats_matching_constant_type_skips` | control: the same column skips when the constant matches its type | -| `test_export_stats_cross_type_constant_skips_nothing` | a cross-type constant skips nothing, on the identical predicate | +| `test_export_stats_cross_type_constant_skips_like_the_matching_one` | a cross-type constant skips as the matching one does, on the identical predicate (since #1712; it skipped nothing) | | `test_export_stats_float_less_than_skips` | control: a float < predicate below every bound skips | | `test_export_stats_float_greater_equal_skips_nothing` | a float >= predicate above every bound still skips nothing | | `test_export_stats_quoted_bigint_literal_skips` | a quoted literal takes a bigint column's type and skips | -| `test_export_stats_unquoted_bigint_literal_skips_nothing` | the same digits unquoted are an integer, and skip nothing | +| `test_export_stats_unquoted_bigint_literal_skips_as_well` | the same digits unquoted are an integer, and skip as well | | `test_export_stats_wide_digits_skip_a_bigint` | digits too wide for an integer are a bigint, and skip | | `test_export_stats_decimal_literal_casts_the_column` | a decimal-point literal casts the column, not the constant | | `test_export_stats_quoted_literal_casts_the_constant` | control: the quoted form casts the constant to the column's type | | `test_export_stats_quoted_real_literal_skips` | a quoted literal takes a real column's type and skips | -| `test_export_stats_unquoted_real_literal_skips_nothing` | the same literal unquoted is double precision, and skips nothing | +| `test_export_stats_unquoted_real_literal_skips_as_well` | the same literal unquoted is double precision, and skips as well | | `test_export_stats_quoted_smallint_literal_skips` | a quoted literal takes a smallint column's type and skips | -| `test_export_stats_unquoted_smallint_literal_skips_nothing` | the same digits unquoted are an integer, and skip nothing (int2) | +| `test_export_stats_unquoted_smallint_literal_skips_as_well` | the same digits unquoted are an integer, and skip as well (int2) | | `test_export_stats_adorned_date_skips` | an adorned literal of the column's own type skips | | `test_export_stats_unadorned_date_skips` | the same date unadorned is unknown, takes the column's type and skips | -| `test_export_stats_timestamp_adorned_date_skips_nothing` | the same date adorned as a timestamp does not skip | +| `test_export_stats_timestamp_adorned_date_skips_as_well` | the same date adorned as a timestamp skips as well | | `test_export_stats_adorned_literal_keeps_its_type` | an adorned literal keeps its own type, not the column's | | `test_export_stats_unadorned_literal_takes_the_column_type` | control: the unadorned form resolves to the column's type instead | | `test_export_stats_integer_digits_prune_every_group` | control: a digit string that fits an integer prunes every group | -| `test_export_stats_int4_column_refuses_wide_digits` | digits too wide for an integer are a bigint, so an int4 column refuses | -| `test_export_stats_fdw_result_equals_the_oracle` | the FDW result equals the oracle for [P]: 20 tests, one per predicate the skip checks use | +| `test_export_stats_int4_column_skips_wide_digits` | digits too wide for an integer are a bigint, and an int4 column skips as well | +| `test_export_stats_fdw_result_equals_the_oracle` | the FDW result equals the oracle for [P]: 23 tests, one per predicate the skip checks use | | `test_export_stats_parser_finds_pyarrow_bounds` | control: the parser finds pyarrow's bounds | | `test_export_stats_parser_reports_deprecated_fields` | control: the parser reports the deprecated fields where they exist | @@ -12872,3 +12873,21 @@ The pytest twin of `test/parquet_timestamp_zone.sh` (#1706 item 2), every check | `test_ptz_a_fall_back_wall_time_is_what_the_cast_gives` | a naive wall time in the fall-back hour read into timestamptz is what the cast gives | | `test_ptz_a_legacy_file_into_timestamp_is_the_session_wall_time` | a legacy TIMESTAMP_MICROS file read into timestamp is the session-zone wall time of its instant | | `test_ptz_a_legacy_file_into_timestamptz_keeps_its_instant` | a legacy TIMESTAMP_MICROS file read into timestamptz keeps its instant | + +## 344. test_parquet_skip_cross_type.py: Parquet row-group skipping uses a constant of another type in the operator family + +The pytest twin of `test/parquet_skip_cross_type.sh` (#1712), every check by the shell's name. Over a file of 200,000 ascending rows in 4 row groups, each arm reads the skip count from EXPLAIN (ANALYZE): an int4 constant on the int8 column (greater-than, equality, and on the left of the operator), and a float4 constant below every row on the float8 column; the answers are compared with the same query in the column's own type, and a float4 NaN constant must still find a NaN row its group's statistics leave out. Before #1712 a constant of another type skipped nothing. The files are written by `python3` with pyarrow, so no test imports pyarrow. On a private database. + +### Every test + +| test | what it holds | +| --- | --- | +| `test_pxt_the_file_holds_four_row_groups` | premise: the file holds 4 row groups (pyarrow's own count) | +| `test_pxt_an_int8_constant_skips_three_groups` | control: an int8 constant on the int8 column skips 3 of the 4 row groups | +| `test_pxt_an_int4_constant_skips_three_groups` | an int4 constant on the int8 column skips 3 of the 4 row groups | +| `test_pxt_the_int4_constant_returns_the_int8_rows` | and returns the rows the int8 constant returns | +| `test_pxt_an_int4_equality_skips_three_groups` | an int4 constant in an equality on the int8 column skips 3 of the 4 row groups | +| `test_pxt_a_constant_on_the_left_is_compared_the_same_way` | a constant on the left of the operator is compared the same way | +| `test_pxt_a_float4_constant_below_every_row_skips_all_groups` | a float4 constant below every row skips all 4 row groups of the float8 column -- negative, because a float4 misread as float8 is a tiny positive number | +| `test_pxt_the_float4_constant_returns_the_float8_rows` | and returns the rows the float8 constant returns | +| `test_pxt_a_float4_nan_still_finds_the_nan_row` | a float4 NaN constant still finds the float8 column's NaN row | diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 5e30d3814..1b54a4ae5 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -1212,4 +1212,8 @@ guard_tests 839 # one structural test, by collection over the cluster files. # cluster_tests 4231 -> 4249 for #1706: test_parquet_timestamp_zone.py, the twin of parquet_timestamp_zone.sh # (16 tests, one per check), and two mixed-kind hour() arms in test_iceberg_fdw.py, by collection. -cluster_tests 4249 +# cluster_tests 4249 -> 4258 for #1712's test_parquet_skip_cross_type.py, the twin of parquet_skip_cross_type.sh: +# 9 tests, one per check, by collection over the cluster files. +# cluster_tests 4258 -> 4261 for #1712's three soundness arms in test_parquet_export_stats.py +# (parametrized), by collection over the cluster files. +cluster_tests 4261 diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index 834787edf..4093a5795 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -207,6 +207,7 @@ "cancel_decode", "bench_guards", "catalog_delete_index", + "parquet_skip_cross_type", "parquet_timestamp_zone", "catalog_filter_reads", "catalog_plan_index", diff --git a/test/pytest/test_parquet_export_stats.py b/test/pytest/test_parquet_export_stats.py index a980ce862..6949cd207 100644 --- a/test/pytest/test_parquet_export_stats.py +++ b/test/pytest/test_parquet_export_stats.py @@ -144,6 +144,9 @@ ("id < -1", "the FDW result equals the oracle for [id < -1]"), ("id < -5000000000", "the FDW result equals the oracle for [id < -5000000000]"), ("alln IS NULL", "the FDW result equals the oracle for [alln IS NULL]"), + ("i8 < 5::int", "the FDW result equals the oracle for [i8 < 5::int]"), + ("i8 < 5", "the FDW result equals the oracle for [i8 < 5]"), + ("f4 < 0.4", "the FDW result equals the oracle for [f4 < 0.4]"), ] @@ -745,9 +748,10 @@ def test_export_stats_matching_constant_type_skips(es, expect): "control: the same column skips when the constant matches its type") -def test_export_stats_cross_type_constant_skips_nothing(es, expect): - expect.num(es["skipped"]["WHERE i8 < 5::int"], 0, - "a cross-type constant skips nothing, on the identical predicate") +def test_export_stats_cross_type_constant_skips_like_the_matching_one(es, expect): + # Since #1712 a constant of another type in the family skips (was: nothing). + expect.num(es["skipped"]["WHERE i8 < 5::int"], NGROUPS, + "a cross-type constant skips as the matching one does, on the identical predicate") def test_export_stats_float_less_than_skips(es, expect): @@ -765,9 +769,9 @@ def test_export_stats_quoted_bigint_literal_skips(es, expect): "a quoted literal takes a bigint column's type and skips") -def test_export_stats_unquoted_bigint_literal_skips_nothing(es, expect): - expect.num(es["skipped"]["WHERE i8 < 5"], 0, - "the same digits unquoted are an integer, and skip nothing") +def test_export_stats_unquoted_bigint_literal_skips_as_well(es, expect): + expect.num(es["skipped"]["WHERE i8 < 5"], NGROUPS, + "the same digits unquoted are an integer, and skip as well") def test_export_stats_wide_digits_skip_a_bigint(es, expect): @@ -790,9 +794,9 @@ def test_export_stats_quoted_real_literal_skips(es, expect): "a quoted literal takes a real column's type and skips") -def test_export_stats_unquoted_real_literal_skips_nothing(es, expect): - expect.num(es["skipped"]["WHERE f4 < 0.4"], 0, - "the same literal unquoted is double precision, and skips nothing") +def test_export_stats_unquoted_real_literal_skips_as_well(es, expect): + expect.num(es["skipped"]["WHERE f4 < 0.4"], NGROUPS, + "the same literal unquoted is double precision, and skips as well") def test_export_stats_quoted_smallint_literal_skips(es, expect): @@ -800,9 +804,9 @@ def test_export_stats_quoted_smallint_literal_skips(es, expect): "a quoted literal takes a smallint column's type and skips") -def test_export_stats_unquoted_smallint_literal_skips_nothing(es, expect): - expect.num(es["skipped"]["WHERE i2 < 500"], 0, - "the same digits unquoted are an integer, and skip nothing (int2)") +def test_export_stats_unquoted_smallint_literal_skips_as_well(es, expect): + expect.num(es["skipped"]["WHERE i2 < 500"], 1, + "the same digits unquoted are an integer, and skip as well (int2)") def test_export_stats_adorned_date_skips(es, expect): @@ -815,9 +819,9 @@ def test_export_stats_unadorned_date_skips(es, expect): "the same date unadorned is unknown, takes the column's type and skips") -def test_export_stats_timestamp_adorned_date_skips_nothing(es, expect): - expect.num(es["skipped"]["WHERE d < TIMESTAMP '2000-01-05 00:00:00'"], 0, - "the same date adorned as a timestamp does not skip") +def test_export_stats_timestamp_adorned_date_skips_as_well(es, expect): + expect.num(es["skipped"]["WHERE d < TIMESTAMP '2000-01-05 00:00:00'"], 3, + "the same date adorned as a timestamp skips as well") def test_export_stats_adorned_literal_keeps_its_type(es, expect): @@ -835,9 +839,9 @@ def test_export_stats_integer_digits_prune_every_group(es, expect): "control: a digit string that fits an integer prunes every group") -def test_export_stats_int4_column_refuses_wide_digits(es, expect): - expect.num(es["skipped"]["WHERE id < -5000000000"], 0, - "digits too wide for an integer are a bigint, so an int4 column refuses") +def test_export_stats_int4_column_skips_wide_digits(es, expect): + expect.num(es["skipped"]["WHERE id < -5000000000"], NGROUPS, + "digits too wide for an integer are a bigint, and an int4 column skips as well") @pytest.mark.parametrize("pred,name", SOUND) diff --git a/test/pytest/test_parquet_skip_cross_type.py b/test/pytest/test_parquet_skip_cross_type.py new file mode 100644 index 000000000..015a80d31 --- /dev/null +++ b/test/pytest/test_parquet_skip_cross_type.py @@ -0,0 +1,175 @@ +"""Parquet row-group skipping uses a constant of another type in the column's operator +family (#1712). The pytest twin of `test/parquet_skip_cross_type.sh`, every check by the +shell's name. + +The group-exclusion check refused any constant whose type was not the column's, so +`WHERE id > 150000` (an int4 literal) on a bigint column read every row group. Over a +file of 200,000 ascending rows in 4 row groups, each arm reads the skip count from +EXPLAIN (ANALYZE): int4 constants on the int8 column (greater-than, equality, and the +constant on the left), a float4 constant on the float8 column, the answers compared with +the same query written in the column's own type, and a float4 NaN constant that must +still find the float8 column's NaN row. + +The files are written by `python3` with pyarrow, as the shell writes them, so no test +imports pyarrow: each declines by the shell's skip name first, then asks for the module +fixture. ON A PRIVATE DATABASE. +""" +import re +import subprocess +import sys + +import psycopg +import pytest + +from pgc_vacuity import query_error + +DB = "pgc_parquet_skip_cross_type" + +WRITER = r''' +import sys, pyarrow as pa, pyarrow.parquet as pq +w = sys.argv[1] +ids = list(range(200000)) +pq.write_table(pa.table({"id": pa.array(ids, pa.int64()), + "f": pa.array([float(i) for i in ids], pa.float64())}), + f"{w}/xt.parquet", row_group_size=50000) +pq.write_table(pa.table({"f": pa.array([1.0, 2.0, 3.0, float("nan")], pa.float64())}), + f"{w}/xn.parquet", row_group_size=2) +print(pq.ParquetFile(f"{w}/xt.parquet").metadata.num_row_groups) +''' + + +def _failed(exc): + return query_error(str(exc).splitlines()[0] if str(exc) else type(exc).__name__) + + +def _value(dsn, sql): + """-> the first column of the first row, or query_error() carrying the failure.""" + try: + with psycopg.connect(dsn, autocommit=True) as conn: + row = conn.execute(sql).fetchone() + except psycopg.Error as exc: + return _failed(exc) + return None if row is None else row[0] + + +def _run(dsn, sql): + """-> None, or query_error() when the statement failed.""" + try: + with psycopg.connect(dsn, autocommit=True) as conn: + conn.execute(sql) + except psycopg.Error as exc: + return _failed(exc) + return None + + +def _skipped(dsn, query): + """-> the "Row Groups Skipped" count of EXPLAIN (ANALYZE), or query_error().""" + try: + with psycopg.connect(dsn, autocommit=True) as conn: + plan = "\n".join(r[0] for r in conn.execute( + f"EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF) {query}").fetchall()) + except psycopg.Error as exc: + return _failed(exc) + m = re.search(r"Row Groups Skipped: (\d+)", plan) + return int(m.group(1)) if m else query_error(f"no skip count in: {plan.splitlines()[:1]}") + + +@pytest.fixture(scope="module") +def pxt(pgc_cluster, server_dir): + with psycopg.connect(pgc_cluster.dsn(), autocommit=True) as a: + a.execute(f"DROP DATABASE IF EXISTS {DB} WITH (FORCE)") + a.execute(f"CREATE DATABASE {DB}") + try: + dsn = pgc_cluster.dsn(dbname=DB) + w = server_dir + wrote = subprocess.run([sys.executable, "-c", WRITER, str(w)], capture_output=True, text=True) + for f in ("xt", "xn"): + if (w / f"{f}.parquet").exists(): + (w / f"{f}.parquet").chmod(0o644) + r = {"groups": int(wrote.stdout.strip()) if wrote.returncode == 0 and wrote.stdout.strip().isdigit() + else query_error(f"the writer failed: {wrote.stderr.strip().splitlines()[-1:]}")} + # A failed statement here makes every later reading a query_error, which + # reddens its check by name rather than erroring the fixture. + for sql in ("CREATE EXTENSION IF NOT EXISTS pgcolumnar", + "CREATE SERVER pq FOREIGN DATA WRAPPER pgcolumnar_parquet", + f"CREATE FOREIGN TABLE xt (id int8, f float8) SERVER pq OPTIONS (path '{w}/xt.parquet')", + f"CREATE FOREIGN TABLE xn (f float8) SERVER pq OPTIONS (path '{w}/xn.parquet')"): + _run(dsn, sql) + r["int8 skip"] = _skipped(dsn, "SELECT count(*) FROM xt WHERE id > 150000::int8") + r["int4 skip"] = _skipped(dsn, "SELECT count(*) FROM xt WHERE id > 150000") + r["int4 rows"] = _value(dsn, "SELECT count(*) FROM xt WHERE id > 150000") + r["int8 rows"] = _value(dsn, "SELECT count(*) FROM xt WHERE id > 150000::int8") + r["int4 eq skip"] = _skipped(dsn, "SELECT count(*) FROM xt WHERE id = 150001") + r["left skip"] = _skipped(dsn, "SELECT count(*) FROM xt WHERE 150000 < id") + r["float4 skip"] = _skipped(dsn, "SELECT count(*) FROM xt WHERE f < -1::float4") + r["float4 rows"] = _value(dsn, "SELECT count(*) FROM xt WHERE f < 50000::float4") + r["float8 rows"] = _value(dsn, "SELECT count(*) FROM xt WHERE f < 50000::float8") + r["nan rows"] = _value(dsn, "SELECT count(*) FROM xn WHERE f = 'NaN'::float4") + yield r + finally: + with psycopg.connect(pgc_cluster.dsn(), autocommit=True) as a: + a.execute(f"DROP DATABASE IF EXISTS {DB} WITH (FORCE)") + + +def _readings(request, expect): + """Decline by the shell's skip name first; only then build the fixture.""" + if expect.require_module("pyarrow.parquet", + name="pyarrow not available; Parquet cross-type skip suite needs it") is None: + return None + return request.getfixturevalue("pxt") + + +def test_pxt_the_file_holds_four_row_groups(request, expect): + r = _readings(request, expect) + if r is not None: + expect.num(r["groups"], 4, "premise: the file holds 4 row groups (pyarrow's own count)") + + +def test_pxt_an_int8_constant_skips_three_groups(request, expect): + r = _readings(request, expect) + if r is not None: + expect.num(r["int8 skip"], 3, "control: an int8 constant on the int8 column skips 3 of the 4 row groups") + + +def test_pxt_an_int4_constant_skips_three_groups(request, expect): + r = _readings(request, expect) + if r is not None: + expect.num(r["int4 skip"], 3, "an int4 constant on the int8 column skips 3 of the 4 row groups") + + +def test_pxt_the_int4_constant_returns_the_int8_rows(request, expect): + r = _readings(request, expect) + if r is not None: + expect.num(r["int4 rows"], r["int8 rows"], "and returns the rows the int8 constant returns") + + +def test_pxt_an_int4_equality_skips_three_groups(request, expect): + r = _readings(request, expect) + if r is not None: + expect.num(r["int4 eq skip"], 3, + "an int4 constant in an equality on the int8 column skips 3 of the 4 row groups") + + +def test_pxt_a_constant_on_the_left_is_compared_the_same_way(request, expect): + r = _readings(request, expect) + if r is not None: + expect.num(r["left skip"], 3, "a constant on the left of the operator is compared the same way") + + +def test_pxt_a_float4_constant_below_every_row_skips_all_groups(request, expect): + r = _readings(request, expect) + if r is not None: + expect.num(r["float4 skip"], 4, + "a float4 constant below every row skips all 4 row groups of the float8 column") + + +def test_pxt_the_float4_constant_returns_the_float8_rows(request, expect): + r = _readings(request, expect) + if r is not None: + expect.num(r["float4 rows"], r["float8 rows"], "and returns the rows the float8 constant returns") + + +def test_pxt_a_float4_nan_still_finds_the_nan_row(request, expect): + r = _readings(request, expect) + if r is not None: + expect.num(r["nan rows"], 1, "a float4 NaN constant still finds the float8 column's NaN row") diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 5d7c24419..609fa168d 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -261,6 +261,7 @@ SUITES=( parquet_level_width parquet_nested parquet_nested_import + parquet_skip_cross_type parquet_timestamp_zone partition_lifecycle partition_rows From 26fb02f2036c673912f0445cbef41cfe1f1a6432 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 22 Oct 2026 19:13:28 +0000 Subject: [PATCH 3/7] fix: Parquet skipping never compares a timestamp column with a timestamptz constant (#1712) OffgridwithJD's review of #1737 measured a wrong answer. For a timestamp column and a timestamptz constant, timestamp_cmp_timestamptz turns each statistic, a wall time, into an instant in the session zone, and that is not monotonic across a spring-forward gap. In New York a row group holding 2024-03-10 02:00, 02:30 and 03:00 has a converted max of 07:00Z but holds 02:30, which is 07:30Z. So `ts > '2024-03-10 07:15+00'::timestamptz` skipped the group and returned no row; a heap copy returns row 1. The same held for >= 07:15Z and = 07:30Z. The group-exclusion check now refuses that pair before choosing the comparison function. A timestamp constant on a timestamp column still skips, and a timestamptz column with a timestamp constant is unchanged. docs/limitations.md and the CHANGELOG entry say so. parquet_skip_cross_type and its twin gain 7 checks over a naive timestamp file in 2 row groups, every session in New York: the three predicates against the heap copy, a same-type control that still skips 1 group, its answer against the heap, and two premises. Measured: - Red: on the rebased head without the guard, PG17, the >, >= and = arms fail by the same names in shell (13 passed + 3 failed = 16) and twin (3 failed, 13 passed), each "got none want 1". - Mutant: deleting the guard line reddens the same three names in both harnesses, which dates their ledger rows. - Green: parquet_skip_cross_type 16/16 and parquet_export_stats 165/165, shell and twin, on each of PG15 to PG19. parquet_timestamp_zone 16/16 shell and twin on PG17. - Guards 804 passed. docs_style passes. cluster_tests 3252 -> 3259 (collected 3259). Ledger 6963 + 7 = 6970 rows, 4146 -> 4150 never, 2817 -> 2820 dated; the gate passes against origin/main, ceiling 75. compare_to_bash: every bash property is covered. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 6 +- docs/limitations.md | 5 +- src/columnar_parquet_reader.c | 10 +++ test/check_ledger.tsv | 7 ++ test/check_ledger_budget.txt | 17 +++- test/parquet_skip_cross_type.sh | 37 +++++++- test/pytest/TESTS.md | 9 +- test/pytest/expected_tests.txt | 4 +- test/pytest/test_parquet_skip_cross_type.py | 97 ++++++++++++++++++--- 9 files changed, 174 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20b087f0b..50fa74a13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -449,8 +449,10 @@ true until the next version shipped. column of a Parquet foreign table read every row group; only `150000::int8` skipped. The check now compares through the operator family's cross-type comparison, as the native columnar scan already did. Answers were always correct; - this only reads less (#1712). `test/parquet_skip_cross_type.sh` and - `test_parquet_skip_cross_type.py`. + this only reads less (#1712). A `timestamp` column compared with a `timestamptz` + constant still does not skip: across a spring-forward gap the session zone's + conversion does not keep the statistics' order. `test/parquet_skip_cross_type.sh` + and `test_parquet_skip_cross_type.py`. - **`export_parquet` no longer marks a `timestamp` column as a UTC instant.** Both `timestamp` and `timestamptz` were written as the converted type `TIMESTAMP_MICROS`, which the Parquet spec defines as adjusted to UTC. Every other reader (pyarrow, diff --git a/docs/limitations.md b/docs/limitations.md index 2b657adc0..243816e63 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -1095,7 +1095,10 @@ correct rows; these are the conditions under which it can skip at all: `bigint_col > 5` skips, though `5` is an `int4`. So do `real_col < 0.4` and `ts >= DATE '2026-01-01'` on a `timestamp` column. A comparison that casts the column instead does not skip. An example is `bigint_col < 5.0`, where `5.0` is a - `numeric`. + `numeric`. A `timestamp` column compared with a `timestamptz` constant does not + skip either. The comparison turns each wall time into an instant in the session's + time zone. Across a spring-forward gap that does not keep their order, so the + statistics would not bound the rows. - The row group's statistics must carry both a minimum and a maximum, and the interval must not be inverted. Two cases decode to an interval that the reader does not trust for skipping. The first is an unsigned Parquet column that diff --git a/src/columnar_parquet_reader.c b/src/columnar_parquet_reader.c index 6a6f1c363..a47601d97 100644 --- a/src/columnar_parquet_reader.c +++ b/src/columnar_parquet_reader.c @@ -4571,7 +4571,17 @@ pqfdw_clause_excludes_group(ImpLeaf *leaf, PqChunk *ch, int strategy, * path does (#1712). A bare integer literal is int4, so requiring the column's * own type left `WHERE id > 150000` on a bigint column skipping nothing. A * family with no such function, or no family, means no skip. + * + * Except a timestamp column against a timestamptz constant: the comparison + * converts each statistic, a wall time, to an instant in the session zone, + * which is not monotonic across a spring-forward gap. In New York a group + * holding 02:00, 02:30 and 03:00 has max 07:00Z but a row at 07:30Z, so + * `ts > '07:15+00'` would skip it (OffgridwithJD's review of #1737). Never + * skip that pair. A timestamptz column and a timestamp constant is safe: the + * statistics are instants, and only the one constant is converted. */ + if (leaf->plan.typid == TIMESTAMPOID && con->consttype == TIMESTAMPTZOID) + return false; if (con->consttype == leaf->plan.typid) vsConst = &tce->cmp_proc_finfo; else diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index b1e1daace..93e0c6266 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -5810,12 +5810,19 @@ parquet_nested_import parquet_nested_import round-trips after delete 15;16;17;18 parquet_skip_cross_type parquet_skip_cross_type a constant on the left of the operator is compared the same way 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the comparison function is called with the constant first, not the column's value parquet_skip_cross_type parquet_skip_cross_type a float4 NaN constant still finds the float8 column's NaN row 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the float NaN check reads the constant as the column's float type parquet_skip_cross_type parquet_skip_cross_type a float4 constant below every row skips all 4 row groups of the float8 column 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the column type's own comparison function is used for a constant of another type +parquet_skip_cross_type parquet_skip_cross_type a timestamptz constant after a spring-forward gap finds the timestamp column's row the heap finds (>) 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: a timestamp column is compared with a timestamptz constant through the cross-type function +parquet_skip_cross_type parquet_skip_cross_type a timestamptz constant after a spring-forward gap finds the timestamp column's row the heap finds (>=) 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: a timestamp column is compared with a timestamptz constant through the cross-type function +parquet_skip_cross_type parquet_skip_cross_type a timestamptz constant equal to a row's instant after a spring-forward gap finds that row, as the heap does 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: a timestamp column is compared with a timestamptz constant through the cross-type function parquet_skip_cross_type parquet_skip_cross_type an int4 constant in an equality on the int8 column skips 3 of the 4 row groups 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the comparison function is called with the constant first, not the column's value parquet_skip_cross_type parquet_skip_cross_type an int4 constant on the int8 column skips 3 of the 4 row groups 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the comparison function is called with the constant first, not the column's value parquet_skip_cross_type parquet_skip_cross_type and returns the rows the float8 constant returns 15;16;17;18;19 never - +parquet_skip_cross_type parquet_skip_cross_type and returns the rows the heap returns 15;16;17;18;19 never - parquet_skip_cross_type parquet_skip_cross_type and returns the rows the int8 constant returns 15;16;17;18;19 never - +parquet_skip_cross_type parquet_skip_cross_type control: a timestamp constant on the timestamp column skips 1 of the 2 row groups 15;16;17;18;19 never - parquet_skip_cross_type parquet_skip_cross_type control: an int8 constant on the int8 column skips 3 of the 4 row groups 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the comparison function is called with the constant first, not the column's value +parquet_skip_cross_type parquet_skip_cross_type premise: in New York the heap copy finds row 1 for ts > 07:15Z, ts >= 07:15Z and ts = 07:30Z 15;16;17;18;19 never - parquet_skip_cross_type parquet_skip_cross_type premise: the file holds 4 row groups (pyarrow's own count) 15;16;17;18;19 never - +parquet_skip_cross_type parquet_skip_cross_type premise: the gap file holds 2 row groups (pyarrow's own count) 15;16;17;18;19 never - parquet_timestamp_zone parquet_timestamp_zone a legacy TIMESTAMP_MICROS file read into timestamp is the session-zone wall time of its instant 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: a column with only a legacy TIMESTAMP_* converted type reads as not UTC-adjusted parquet_timestamp_zone parquet_timestamp_zone a legacy TIMESTAMP_MICROS file read into timestamptz keeps its instant 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: a column with only a legacy TIMESTAMP_* converted type reads as not UTC-adjusted parquet_timestamp_zone parquet_timestamp_zone a naive file read into timestamptz in Tokyo is its wall time there, as the cast gives 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: a naive column read into timestamptz is converted in America/New_York, not the session zone diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 51327c27c..e332815a2 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -7099,4 +7099,19 @@ suites_not_covered 27 # never 4541 + 3 = 4544 # dated 3464 + 0 = 3464 # 4544 + 3464 == 8008 -checks_never_observed_red 4544 +# +# #1712, parquet_skip_cross_type's spring-forward arms, with its twin (jdatcmd): 4544 -> 4548. +# OffgridwithJD's review measured a timestamp column against a timestamptz constant: the +# cross-type comparison converts each wall-time statistic to a session-zone instant, which +# is not monotonic across a spring-forward gap, so in New York `ts > '2024-03-10 07:15+00'` +# skipped the group holding 02:30 (07:30Z). The reader now refuses that pair. 7 checks are +# added, green on each of 15-19 in shell and twin alike. 3 are dated by one mutation, the +# guard deleted, which reddens the same three names in both twins: the >, >= and = arms +# against the heap copy. The two premises, the same-type skip control and its answer +# arrive never. suites_not_covered does not move. +# +# ledger rows 8008 + 7 = 8015 +# never 4544 + 4 = 4548 +# dated 3464 + 3 = 3467 +# 4548 + 3467 == 8015 +checks_never_observed_red 4548 diff --git a/test/parquet_skip_cross_type.sh b/test/parquet_skip_cross_type.sh index 3f61c1ada..e3e4d119a 100755 --- a/test/parquet_skip_cross_type.sh +++ b/test/parquet_skip_cross_type.sh @@ -35,10 +35,19 @@ pq.write_table(pa.table({"id": pa.array(ids, pa.int64()), # NaN sits in the second group; Parquet statistics leave it out of min and max pq.write_table(pa.table({"f": pa.array([1.0, 2.0, 3.0, float("nan")], pa.float64())}), f"{w}/xn.parquet", row_group_size=2) -print(pq.ParquetFile(f"{w}/xt.parquet").metadata.num_row_groups) +# a naive timestamp column in 2 row groups: group 0 spans New York's 2024-03-10 +# spring-forward gap (ids 0-2), group 1 is the hour before it (ids 3-5) +import datetime as dt +gap = [dt.datetime(2024, 3, 10, h, m) for h, m in ((2, 0), (2, 30), (3, 0), (1, 0), (1, 10), (1, 20))] +pq.write_table(pa.table({"id": pa.array(range(6), pa.int64()), "ts": pa.array(gap, pa.timestamp("us"))}), + f"{w}/xg.parquet", row_group_size=3) +print(pq.ParquetFile(f"{w}/xt.parquet").metadata.num_row_groups, + pq.ParquetFile(f"{w}/xg.parquet").metadata.num_row_groups) PY )" -chmod 644 "$W/xt.parquet" "$W/xn.parquet" +GGROUPS="${NGROUPS#* }" +NGROUPS="${NGROUPS%% *}" +chmod 644 "$W/xt.parquet" "$W/xn.parquet" "$W/xg.parquet" psql_run "CREATE SERVER pq FOREIGN DATA WRAPPER pgcolumnar_parquet; CREATE FOREIGN TABLE xt (id int8, f float8) SERVER pq OPTIONS (path '$W/xt.parquet'); CREATE FOREIGN TABLE xn (f float8) SERVER pq OPTIONS (path '$W/xn.parquet');" @@ -74,4 +83,28 @@ check_num "and returns the rows the float8 constant returns" \ check_num "a float4 NaN constant still finds the float8 column's NaN row" \ "$(q "SELECT count(*) FROM xn WHERE f = 'NaN'::float4;")" "1" +# A timestamp column and a timestamptz constant (OffgridwithJD's review of #1737). +# The cross-type comparison converts each statistic, a wall time, to an instant in +# the session zone, and that conversion is not monotonic across a spring-forward +# gap. In New York, group 0's max 03:00 becomes 07:00Z, yet its row 02:30 becomes +# 07:30Z. Every session below runs in New York; the heap copy is the oracle. +psql_run "CREATE FOREIGN TABLE xg (id int8, ts timestamp) SERVER pq OPTIONS (path '$W/xg.parquet'); + CREATE TABLE xg_heap AS SELECT * FROM xg;" +qny() { PGOPTIONS="-c TimeZone=America/New_York" q "$1"; } +ids() { qny "SELECT coalesce(string_agg(id::text, ',' ORDER BY id), 'none') FROM $1 WHERE $2;"; } +check_num "premise: the gap file holds 2 row groups (pyarrow's own count)" "$GGROUPS" "2" +check_text "premise: in New York the heap copy finds row 1 for ts > 07:15Z, ts >= 07:15Z and ts = 07:30Z" \ + "$(ids xg_heap "ts > '2024-03-10 07:15+00'::timestamptz") $(ids xg_heap "ts >= '2024-03-10 07:15+00'::timestamptz") $(ids xg_heap "ts = '2024-03-10 07:30+00'::timestamptz")" \ + "1 1 1" +check_text "a timestamptz constant after a spring-forward gap finds the timestamp column's row the heap finds (>)" \ + "$(ids xg "ts > '2024-03-10 07:15+00'::timestamptz")" "$(ids xg_heap "ts > '2024-03-10 07:15+00'::timestamptz")" +check_text "a timestamptz constant after a spring-forward gap finds the timestamp column's row the heap finds (>=)" \ + "$(ids xg "ts >= '2024-03-10 07:15+00'::timestamptz")" "$(ids xg_heap "ts >= '2024-03-10 07:15+00'::timestamptz")" +check_text "a timestamptz constant equal to a row's instant after a spring-forward gap finds that row, as the heap does" \ + "$(ids xg "ts = '2024-03-10 07:30+00'::timestamptz")" "$(ids xg_heap "ts = '2024-03-10 07:30+00'::timestamptz")" +check_num "control: a timestamp constant on the timestamp column skips 1 of the 2 row groups" \ + "$(PGOPTIONS="-c TimeZone=America/New_York" skipped "SELECT count(*) FROM xg WHERE ts > '2024-03-10 02:00'::timestamp")" "1" +check_text "and returns the rows the heap returns" \ + "$(ids xg "ts > '2024-03-10 02:00'::timestamp")" "$(ids xg_heap "ts > '2024-03-10 02:00'::timestamp")" + pgc_summary diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index d17facb4a..139e73e45 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -12876,7 +12876,7 @@ The pytest twin of `test/parquet_timestamp_zone.sh` (#1706 item 2), every check ## 344. test_parquet_skip_cross_type.py: Parquet row-group skipping uses a constant of another type in the operator family -The pytest twin of `test/parquet_skip_cross_type.sh` (#1712), every check by the shell's name. Over a file of 200,000 ascending rows in 4 row groups, each arm reads the skip count from EXPLAIN (ANALYZE): an int4 constant on the int8 column (greater-than, equality, and on the left of the operator), and a float4 constant below every row on the float8 column; the answers are compared with the same query in the column's own type, and a float4 NaN constant must still find a NaN row its group's statistics leave out. Before #1712 a constant of another type skipped nothing. The files are written by `python3` with pyarrow, so no test imports pyarrow. On a private database. +The pytest twin of `test/parquet_skip_cross_type.sh` (#1712), every check by the shell's name. Over a file of 200,000 ascending rows in 4 row groups, each arm reads the skip count from EXPLAIN (ANALYZE): an int4 constant on the int8 column (greater-than, equality, and on the left of the operator), and a float4 constant below every row on the float8 column; the answers are compared with the same query in the column's own type, and a float4 NaN constant must still find a NaN row its group's statistics leave out. A second file, a naive timestamp column whose first row group spans New York's 2024-03-10 spring-forward gap, is read in New York: a timestamptz constant must find the rows a heap copy finds, because converting wall-time statistics to instants does not keep their order across the gap, and a timestamp constant must still skip. Before #1712 a constant of another type skipped nothing. The files are written by `python3` with pyarrow, so no test imports pyarrow. On a private database. ### Every test @@ -12891,3 +12891,10 @@ The pytest twin of `test/parquet_skip_cross_type.sh` (#1712), every check by the | `test_pxt_a_float4_constant_below_every_row_skips_all_groups` | a float4 constant below every row skips all 4 row groups of the float8 column -- negative, because a float4 misread as float8 is a tiny positive number | | `test_pxt_the_float4_constant_returns_the_float8_rows` | and returns the rows the float8 constant returns | | `test_pxt_a_float4_nan_still_finds_the_nan_row` | a float4 NaN constant still finds the float8 column's NaN row | +| `test_pxt_the_gap_file_holds_two_row_groups` | premise: the gap file holds 2 row groups (pyarrow's own count) | +| `test_pxt_the_heap_finds_row_one_for_each_gap_predicate` | premise: in New York the heap copy finds row 1 for ts > 07:15Z, ts >= 07:15Z and ts = 07:30Z | +| `test_pxt_a_timestamptz_constant_after_a_gap_finds_the_heap_row_gt` | a timestamptz constant after a spring-forward gap finds the timestamp column's row the heap finds (>) | +| `test_pxt_a_timestamptz_constant_after_a_gap_finds_the_heap_row_ge` | a timestamptz constant after a spring-forward gap finds the timestamp column's row the heap finds (>=) | +| `test_pxt_a_timestamptz_equality_after_a_gap_finds_the_heap_row` | a timestamptz constant equal to a row's instant after a spring-forward gap finds that row, as the heap does | +| `test_pxt_a_timestamp_constant_skips_one_gap_group` | control: a timestamp constant on the timestamp column skips 1 of the 2 row groups | +| `test_pxt_the_timestamp_constant_returns_the_heap_rows` | and returns the rows the heap returns | diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 1b54a4ae5..190fe6c22 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -1216,4 +1216,6 @@ guard_tests 839 # 9 tests, one per check, by collection over the cluster files. # cluster_tests 4258 -> 4261 for #1712's three soundness arms in test_parquet_export_stats.py # (parametrized), by collection over the cluster files. -cluster_tests 4261 +# cluster_tests 4261 -> 4268 for #1712's seven spring-forward arms in test_parquet_skip_cross_type.py, +# one per check, by collection over the cluster files. +cluster_tests 4268 diff --git a/test/pytest/test_parquet_skip_cross_type.py b/test/pytest/test_parquet_skip_cross_type.py index 015a80d31..65adf2820 100644 --- a/test/pytest/test_parquet_skip_cross_type.py +++ b/test/pytest/test_parquet_skip_cross_type.py @@ -8,7 +8,10 @@ EXPLAIN (ANALYZE): int4 constants on the int8 column (greater-than, equality, and the constant on the left), a float4 constant on the float8 column, the answers compared with the same query written in the column's own type, and a float4 NaN constant that must -still find the float8 column's NaN row. +still find the float8 column's NaN row. A second file holds a naive timestamp column +whose first row group spans New York's 2024-03-10 spring-forward gap: read in New York, +a timestamptz constant must find what a heap copy finds, and a timestamp constant must +still skip (OffgridwithJD's review of #1737). The files are written by `python3` with pyarrow, as the shell writes them, so no test imports pyarrow: each declines by the shell's skip name first, then asks for the module @@ -34,18 +37,30 @@ f"{w}/xt.parquet", row_group_size=50000) pq.write_table(pa.table({"f": pa.array([1.0, 2.0, 3.0, float("nan")], pa.float64())}), f"{w}/xn.parquet", row_group_size=2) -print(pq.ParquetFile(f"{w}/xt.parquet").metadata.num_row_groups) +import datetime as dt +gap = [dt.datetime(2024, 3, 10, h, m) for h, m in ((2, 0), (2, 30), (3, 0), (1, 0), (1, 10), (1, 20))] +pq.write_table(pa.table({"id": pa.array(range(6), pa.int64()), "ts": pa.array(gap, pa.timestamp("us"))}), + f"{w}/xg.parquet", row_group_size=3) +print(pq.ParquetFile(f"{w}/xt.parquet").metadata.num_row_groups, + pq.ParquetFile(f"{w}/xg.parquet").metadata.num_row_groups) ''' +# The gap arms run in New York, as the shell's do (OffgridwithJD's review of #1737). +ZONE = "-c TimeZone=America/New_York" +GT = "ts > '2024-03-10 07:15+00'::timestamptz" +GE = "ts >= '2024-03-10 07:15+00'::timestamptz" +EQ = "ts = '2024-03-10 07:30+00'::timestamptz" +SAME = "ts > '2024-03-10 02:00'::timestamp" + def _failed(exc): return query_error(str(exc).splitlines()[0] if str(exc) else type(exc).__name__) -def _value(dsn, sql): +def _value(dsn, sql, options=""): """-> the first column of the first row, or query_error() carrying the failure.""" try: - with psycopg.connect(dsn, autocommit=True) as conn: + with psycopg.connect(dsn, autocommit=True, options=options) as conn: row = conn.execute(sql).fetchone() except psycopg.Error as exc: return _failed(exc) @@ -62,10 +77,10 @@ def _run(dsn, sql): return None -def _skipped(dsn, query): +def _skipped(dsn, query, options=""): """-> the "Row Groups Skipped" count of EXPLAIN (ANALYZE), or query_error().""" try: - with psycopg.connect(dsn, autocommit=True) as conn: + with psycopg.connect(dsn, autocommit=True, options=options) as conn: plan = "\n".join(r[0] for r in conn.execute( f"EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF) {query}").fetchall()) except psycopg.Error as exc: @@ -83,17 +98,23 @@ def pxt(pgc_cluster, server_dir): dsn = pgc_cluster.dsn(dbname=DB) w = server_dir wrote = subprocess.run([sys.executable, "-c", WRITER, str(w)], capture_output=True, text=True) - for f in ("xt", "xn"): + for f in ("xt", "xn", "xg"): if (w / f"{f}.parquet").exists(): (w / f"{f}.parquet").chmod(0o644) - r = {"groups": int(wrote.stdout.strip()) if wrote.returncode == 0 and wrote.stdout.strip().isdigit() - else query_error(f"the writer failed: {wrote.stderr.strip().splitlines()[-1:]}")} + counts = wrote.stdout.split() + if wrote.returncode == 0 and len(counts) == 2 and all(c.isdigit() for c in counts): + r = {"groups": int(counts[0]), "gap groups": int(counts[1])} + else: + failed = f"the writer failed: {wrote.stderr.strip().splitlines()[-1:]}" + r = {"groups": query_error(failed), "gap groups": query_error(failed)} # A failed statement here makes every later reading a query_error, which # reddens its check by name rather than erroring the fixture. for sql in ("CREATE EXTENSION IF NOT EXISTS pgcolumnar", "CREATE SERVER pq FOREIGN DATA WRAPPER pgcolumnar_parquet", f"CREATE FOREIGN TABLE xt (id int8, f float8) SERVER pq OPTIONS (path '{w}/xt.parquet')", - f"CREATE FOREIGN TABLE xn (f float8) SERVER pq OPTIONS (path '{w}/xn.parquet')"): + f"CREATE FOREIGN TABLE xn (f float8) SERVER pq OPTIONS (path '{w}/xn.parquet')", + f"CREATE FOREIGN TABLE xg (id int8, ts timestamp) SERVER pq OPTIONS (path '{w}/xg.parquet')", + "CREATE TABLE xg_heap AS SELECT * FROM xg"): _run(dsn, sql) r["int8 skip"] = _skipped(dsn, "SELECT count(*) FROM xt WHERE id > 150000::int8") r["int4 skip"] = _skipped(dsn, "SELECT count(*) FROM xt WHERE id > 150000") @@ -105,6 +126,12 @@ def pxt(pgc_cluster, server_dir): r["float4 rows"] = _value(dsn, "SELECT count(*) FROM xt WHERE f < 50000::float4") r["float8 rows"] = _value(dsn, "SELECT count(*) FROM xt WHERE f < 50000::float8") r["nan rows"] = _value(dsn, "SELECT count(*) FROM xn WHERE f = 'NaN'::float4") + for table in ("xg", "xg_heap"): + for name, pred in (("gt", GT), ("ge", GE), ("eq", EQ), ("same", SAME)): + r[f"{table} {name}"] = _value( + dsn, f"SELECT coalesce(string_agg(id::text, ',' ORDER BY id), 'none') FROM {table} WHERE {pred}", + ZONE) + r["same skip"] = _skipped(dsn, f"SELECT count(*) FROM xg WHERE {SAME}", ZONE) yield r finally: with psycopg.connect(pgc_cluster.dsn(), autocommit=True) as a: @@ -173,3 +200,53 @@ def test_pxt_a_float4_nan_still_finds_the_nan_row(request, expect): r = _readings(request, expect) if r is not None: expect.num(r["nan rows"], 1, "a float4 NaN constant still finds the float8 column's NaN row") + + +def test_pxt_the_gap_file_holds_two_row_groups(request, expect): + r = _readings(request, expect) + if r is not None: + expect.num(r["gap groups"], 2, "premise: the gap file holds 2 row groups (pyarrow's own count)") + + +def test_pxt_the_heap_finds_row_one_for_each_gap_predicate(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text((r["xg_heap gt"], r["xg_heap ge"], r["xg_heap eq"]), ("1", "1", "1"), + "premise: in New York the heap copy finds row 1 for ts > 07:15Z, ts >= 07:15Z and ts = 07:30Z") + + +def test_pxt_a_timestamptz_constant_after_a_gap_finds_the_heap_row_gt(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["xg gt"], r["xg_heap gt"], + "a timestamptz constant after a spring-forward gap finds the timestamp column's row " + "the heap finds (>)") + + +def test_pxt_a_timestamptz_constant_after_a_gap_finds_the_heap_row_ge(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["xg ge"], r["xg_heap ge"], + "a timestamptz constant after a spring-forward gap finds the timestamp column's row " + "the heap finds (>=)") + + +def test_pxt_a_timestamptz_equality_after_a_gap_finds_the_heap_row(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["xg eq"], r["xg_heap eq"], + "a timestamptz constant equal to a row's instant after a spring-forward gap finds that row, " + "as the heap does") + + +def test_pxt_a_timestamp_constant_skips_one_gap_group(request, expect): + r = _readings(request, expect) + if r is not None: + expect.num(r["same skip"], 1, + "control: a timestamp constant on the timestamp column skips 1 of the 2 row groups") + + +def test_pxt_the_timestamp_constant_returns_the_heap_rows(request, expect): + r = _readings(request, expect) + if r is not None: + expect.text(r["xg same"], r["xg_heap same"], "and returns the rows the heap returns") From 0aeb420f048e08d9d261c4ba6c96857439a6dd32 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 22 Oct 2026 17:26:13 +0000 Subject: [PATCH 4/7] fix: a fixture port depends on the xdist worker, so -n 2 runs no longer collide (#1742) A twin that starts its own S3 or HTTP fixture server takes the port from pick_port(slot) in a module fixture. Under -n, a module's tests are split across workers, so every worker runs that fixture. The port depended on the slot only, so two workers probed the same free port at the same moment and the second bind failed with "Address already in use" (#1635, #1714, #1721). CI runs the cluster files serially, so CI stayed green; the guard job and developers run -n. pick_port now starts each worker in its own block of 64 ports (WORKER_STRIDE), with the slot as an offset inside it. The worker comes from PYTEST_XDIST_WORKER, so no fixture changes. Slot 0 is the worker's own cluster, which make_cluster now asks for through the same function. A slot that does not fit in a block is refused. The walk on a busy port, the AUX band and the ephemeral-floor contract are unchanged. test_runshape.py gains test_two_workers_taking_a_fixture_port_do_not_collide: an inner -n 2 run where each worker takes pick_port(27), waits until the other has a port, then binds it. On main it is red: "the two workers were given two different ports: got 1 want 2", and the inner bind fails with [Errno 98] Address already in use. With the fix it is green. Measured on PG17 (a copy of the prefix): - test_iceberg_objstore.py under -n 2: main 3 of 3 red ("premise: sigv4 fixture server is up: got '0' want '1'"; "Connection refused"); fixed 16 passed 3 of 3, and 16 passed serially. - test_objstore_listing.py from #1714 under -n 2: as is, 13 failed ("the private cluster did not start"); with this pgc_cluster.py, 15 passed 3 of 3 and serially. - test_connection.py and test_hardening.py under -n 2: 20 passed. guard_tests 476 -> 477, by collection over NO_CLUSTER; TESTS.md section 10 gains a row. The guards (477), test_harness_deps.py and test_capability_decline.py with --pg-config pass over 21182d05. Review (OffgridwithJD): a 64-port block holds slots 0 to 31, and main's twins already ask for 32 to 35 (the stack for up to 41), so slot 32 raised at fixture time: test_iceberg_rest.py "14 errors", ValueError "slot 32 does not fit in a worker's block of 64 ports". WORKER_STRIDE is now 96 (slots 0 to 47; the AUX band's 2000 ports still give 20 workers). Two guards keep the stride and the corpus from drifting apart, red first on the 64 stride: every literal pick_port slot fits a worker's block (it named the 32-35 files), and the corpus's highest slot (35) gets a port in the band. Green: test_runshape.py 18 passed; the five twins that errored pass serially (14, 11, 29, 10, 8); iceberg_rest + iceberg_objstore under -n 2, 30 passed; guards 807 passed on main 9ab6a4cf (guard_tests 804 -> 807). Co-Authored-By: Claude Opus 5.5 --- test/pytest/TESTS.md | 14 ++++ test/pytest/expected_tests.txt | 6 +- test/pytest/pgc_cluster.py | 46 +++++++++++-- test/pytest/test_runshape.py | 114 +++++++++++++++++++++++++++++++++ 4 files changed, 173 insertions(+), 7 deletions(-) diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 139e73e45..8a9b6c02d 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -1694,6 +1694,20 @@ This closes the part of `guard-as-teardown-fixture-still-reports-passed` close the family: a guard anyone adds later in a teardown is still a guard that cannot fail its test, and nothing refuses that shape in general. +### Two workers must not get one fixture port + +A twin that starts its own S3 or HTTP fixture server takes the port from +`pick_port(slot)` in a module fixture. Under `-n`, a module's tests are split across +workers, so every worker runs that fixture. When the port depended on the slot only, +two workers probed the same free port and the second bind failed with `Address +already in use` (#1742). CI runs the cluster files serially, so only `-n` runs met it. + +| test | what it asserts | how it fails | +| --- | --- | --- | +| `test_two_workers_taking_a_fixture_port_do_not_collide` | an inner `-n 2` run where each worker takes `pick_port(27)`, waits for the other, then binds it: two workers, two ports, inner run green | with the port independent of the worker (main before #1742), both workers get one port and the second bind fails | +| `test_every_literal_fixture_slot_fits_a_workers_block` | every literal `pick_port()` in the corpus fits a worker's block of WORKER_STRIDE ports, naming any file that asks for more | +| `test_the_highest_slot_in_the_corpus_gets_a_port` | the corpus's highest slot gets a port inside the AUX band, so the stride and the corpus cannot drift apart | + ## 11. test_zonemap_boundaries.py: exact boundaries ### `test_exact_zonemap_boundaries` diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 190fe6c22..9f27ceaf5 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -326,7 +326,11 @@ # which needs no cluster: 3 tests, by collection over NO_CLUSTER. # guard_tests 804 -> 806 for #1613's test_a_refused_comparison_is_counted_and_named in # test_check_records.py, serial and xdist, by collection over NO_CLUSTER. -guard_tests 839 +# guard_tests 839 -> 840 for #1742's test_two_workers_taking_a_fixture_port_do_not_collide in +# test_runshape.py, by collection over NO_CLUSTER. +# guard_tests 840 -> 842 for #1749's review: every literal fixture slot fits a worker's block, +# and the corpus's highest slot gets a port, in test_runshape.py, by collection over NO_CLUSTER. +guard_tests 842 # The complement: tests that need the driver and a throwaway cluster. Until #1016 these ran # in no CI job at all -- a quarter of the corpus, green when somebody ran them by hand and diff --git a/test/pytest/pgc_cluster.py b/test/pytest/pgc_cluster.py index 8eee9c9b1..4c41e6214 100644 --- a/test/pytest/pgc_cluster.py +++ b/test/pytest/pgc_cluster.py @@ -92,22 +92,56 @@ def port_is_free(port, host="127.0.0.1"): return True -def pick_port(slot, floor=None, is_free=port_is_free): - """A free port in the AUX band for this worker slot. +# THE PORT DEPENDS ON THE WORKER AS WELL AS THE SLOT (#1742). A module fixture that +# starts its own server (an S3 fixture, a private cluster) asks for a fixed slot, +# and under `-n` a module's tests are split across workers, so every worker runs +# that fixture. When the start depended on the slot only, two workers probed the +# same free port at the same moment, and the second bind failed with "Address +# already in use" (#1635, #1714, #1721). Each worker now starts in its own block of +# WORKER_STRIDE ports, and a slot is an offset inside that block. Slot 0 is the +# worker's own cluster (make_cluster); fixtures use 1 and up. +# +# 96, not 64 (#1749 review): main's twins already ask for slots 32 to 35 and the +# stack up to 41, and a block of 64 ports holds slots 0 to 31. 96 holds 0 to 47, and +# the AUX band's 2000 ports still give 20 workers, more than any -n here. The guard +# test_every_literal_fixture_slot_fits_a_workers_block keeps the two from drifting. +WORKER_STRIDE = 96 + + +def worker_slot(worker_id=None): + """The xdist worker's number: 3 for `gw3`, and 0 outside xdist. + + Read from PYTEST_XDIST_WORKER when no id is given, which xdist sets in every + worker, so a fixture calling pick_port needs no argument to be worker-aware. + """ + if worker_id is None: + worker_id = os.environ.get("PYTEST_XDIST_WORKER") + if worker_id in (None, "", "master"): + return 0 + digits = str(worker_id).lstrip("gw") + return int(digits) if digits.isdigit() else 0 + + +def pick_port(slot, floor=None, is_free=port_is_free, worker_id=None): + """A free port in the AUX band for this slot of this worker. Walks on collision instead of trusting the band, because the band only makes a collision unlikely and this harness has already been broken once by a port that something else was holding. """ + if not 0 <= slot * 2 < WORKER_STRIDE: + raise ValueError(f"slot {slot} does not fit in a worker's block of " + f"{WORKER_STRIDE} ports") lo, hi = aux_band(floor if floor is not None else read_ephemeral_floor()) span = hi - lo - start = slot * 2 + start = worker_slot(worker_id) * WORKER_STRIDE + slot * 2 for step in range(span): port = lo + ((start + step) % span) if is_free(port): return port raise RuntimeError( - f"no free port in the AUX band [{lo}, {hi}) for worker slot {slot}") + f"no free port in the AUX band [{lo}, {hi}) for slot {slot} of worker " + f"{worker_slot(worker_id)}") class Cluster: @@ -781,8 +815,8 @@ def make_cluster(pg_config, worker_id): inside `stop()`; failing to stop it must not mask the original error, so the cleanup is itself guarded. """ - slot = 0 if worker_id in (None, "master") else int(str(worker_id).lstrip("gw") or 0) - port = pick_port(slot) + slot = worker_slot(worker_id) + port = pick_port(0, worker_id=worker_id) root = pathlib.Path(tempfile.mkdtemp(prefix=f"pgc-pytest-{slot}-")) cluster = None try: diff --git a/test/pytest/test_runshape.py b/test/pytest/test_runshape.py index 9875b4cd1..0acf0afd2 100644 --- a/test/pytest/test_runshape.py +++ b/test/pytest/test_runshape.py @@ -3,6 +3,9 @@ Each is measured in VACUITY_MODES.md and each is worth more than the per-assertion guards, because a single occurrence silently removes many tests at once. """ +import pathlib +import re + def test_layer_fails_when_a_collected_test_never_reports(pytester, expect): @@ -338,3 +341,114 @@ def test_body(guard_in_teardown, expect): result = pytester.runpytest("-p", "pgc_vacuity") expect.outcomes(result, "a teardown refusal leaves the test reported as passed", passed=1, failed=0, errors=1) + + +def test_two_workers_taking_a_fixture_port_do_not_collide(pytester, monkeypatch, expect): + """Two xdist workers asking `pick_port` for the same slot get different ports (#1742). + + A twin that starts its own S3 or HTTP fixture server takes its port from + `pick_port(slot)` in a module fixture. Under `-n 2` a module's tests are split + across workers, so each worker runs that fixture. On main the port depended on + the slot only: both workers probed the same free port and the second bind failed + with `Address already in use` (#1635, #1714, #1721). CI ran the cluster files + serially, so only the guard job and developers met it. + + The inner tests take their port, wait until both workers have one, and only then + bind it, so the probe-then-bind race happens every run rather than by chance. A + subprocess, because xdist's workers are. + """ + import pathlib + here = pathlib.Path(__file__).resolve().parent + meet = pytester.path / "meet" + meet.mkdir() + monkeypatch.setenv("PYTHONPATH", f"{pytester.path}:{here}") + monkeypatch.setenv("PGC_PORT_MEET", str(meet)) + pytester.makepyfile(test_ports=""" + import os, pathlib, socket, time + from pgc_cluster import pick_port + + MEET = pathlib.Path(os.environ["PGC_PORT_MEET"]) + + def _wait_for(suffix, n): + for _ in range(600): + if len(list(MEET.glob("*" + suffix))) >= n: + return + time.sleep(0.05) + + def _take(expect, me): + port = pick_port(27) + (MEET / (me + ".pick")).write_text( + os.environ.get("PYTEST_XDIST_WORKER", "none") + " " + str(port)) + _wait_for(".pick", 2) + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + try: + s.bind(("127.0.0.1", port)) + s.listen() + got = "bound" + except OSError as e: + got = "bind failed: " + str(e) + (MEET / (me + ".tried")).write_text(got) + _wait_for(".tried", 2) + expect.text(got, "bound", "this worker alone holds port " + str(port)) + + def test_first(expect): + _take(expect, "first") + + def test_second(expect): + _take(expect, "second") + """) + result = pytester.runpytest_subprocess("-p", "pgc_vacuity", "-p", "no:cacheprovider", + "-n", "2") + picks = sorted(p.read_text().split() for p in meet.glob("*.pick")) + workers = sorted({w for w, _ in picks}) + ports = sorted({port for _, port in picks}) + # The premise: the race is only measured if the two picks ran on two workers. + expect.text(" ".join(workers), "gw0 gw1", + "premise: the two inner tests ran on two different workers") + expect.num(len(ports), 2, "the two workers were given two different ports") + out = result.stdout.str() + expect.text("green" if result.ret == 0 else f"exit {result.ret}: " + f"{[l for l in out.splitlines() if 'bind failed' in l or 'FAILED' in l][:4]!r}", + "green", "and each worker bound its own port, so the inner run is green") + + +# ---- every slot the corpus asks for fits a worker's block (#1742 review) ---------- +# +# A slot that does not fit raises at fixture time, on the cluster job, as every test +# of that file erroring. These two read the corpus instead, so the next twin that asks +# for a slot past the block fails here, in the guard job, by name. Measured before +# the stride was raised: main's twins ask for slots 32 to 35, and a 64-port block +# holds 0 to 31. +_SLOT_CALL = re.compile(r"\bpick_port\(\s*(\d+)\s*[,)]") + + +def _corpus_slots(): + here = pathlib.Path(__file__).resolve().parent + found = {} + for f in sorted(here.glob("*.py")): + for m in _SLOT_CALL.finditer(f.read_text()): + found.setdefault(int(m.group(1)), []).append(f.name) + return found + + +def test_every_literal_fixture_slot_fits_a_workers_block(expect): + from pgc_cluster import WORKER_STRIDE + slots = _corpus_slots() + expect.at_least(len(slots), 5, "premise: the corpus scan found the fixture slots in use") + too_big = sorted(f"{s} ({', '.join(sorted(set(files)))})" for s, files in slots.items() + if not s * 2 < WORKER_STRIDE) + expect.text(", ".join(too_big) or "none", "none", + "every literal pick_port slot fits a worker's block of ports") + + +def test_the_highest_slot_in_the_corpus_gets_a_port(expect): + from pgc_cluster import aux_band, pick_port + top = max(_corpus_slots()) + lo, hi = aux_band(32768) + try: + port = pick_port(top, floor=32768, is_free=lambda p: True, worker_id="gw3") + reading = "in the band" if lo <= port < hi else f"outside the band: {port}" + except ValueError as exc: + reading = f"refused: {exc}" + expect.text(reading, "in the band", + f"the highest slot the corpus uses ({top}) gets a port in the AUX band") From cef61cc0529eef689df09d8812e0b647ab0f251b Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 22 Oct 2026 17:41:42 +0000 Subject: [PATCH 5/7] test: parquet_export gains its pytest twin, DuckDB becomes a declinable, pinned reader, and a crash no longer passes as a refusal (#1341) test/pytest/test_parquet_export.py ports all 14 checks of test/parquet_export.sh by name. Five kinds of shell check could pass with nothing measured, or vanish; all are fixed in both twins. - "reject non-columnar table" and "reject unsupported type" passed on any error. With the columnar check removed, exporting a heap failed on "unsupported columnar format version" and passed; with the type refusal formatting the typmod, it failed on "cache lookup failed" and passed. Each refusal now needs its own message. - "parquet values match heap oracle" and "extended values match heap oracle" agreed on two empty readings: with the generator's rows never inserted, both passed. They now need the generator's ids on both sides and the file's row count. - "duckdb non-null count(b)" compared DuckDB's count with t_col, the table under test: with half of t_col deleted before the export, it passed. It is now held to the generator's 80002, by check_num. - Without the duckdb CLI the shell left its four DuckDB checks out without a word (10 = 10). It now skips them by name (10 passed + 4 skipped = 14). A skip, not check_unrunnable: the suites job installs no DuckDB CLI, and an INCOMPLETE suite fails its major there. DuckDB in the twin, as decided on #1341 (option 1 with option 2's mechanism): the duckdb Python package is pinned at 1.5.6 in requirements-test.txt; duckdb joins pyarrow in DECLINABLE_CAPABILITIES, held by four new tests in test_capability_decline.py, written red first; and the cluster job sets PGC_REQUIRE=pyarrow,duckdb. The twin reaches duckdb only through require_module, under the shell's check names. Measured, PG17, shell and twin: - duckdb absent: the four DuckDB tests decline by name and the run is green (10 pass + 4 declined); with PGC_REQUIRE=duckdb the same four fail. - the columnar check removed, and the type refusal formatting the typmod: the refusal red in both; main's shell passed 14 of 14. - the generator's rows never inserted: 8 red in both; main's shell 5. - half of t_col deleted: 4 red in both; main's shell 3. - the type check removed (the backend aborts): the refusal red in both, with the same 5 collateral reds in both. - nine more mutations of src/columnar_parquet.c: each red set identical in both twins and in main's shell. DECIMAL written little-endian and a negative int4 written as null are red in both readers, pyarrow and DuckDB; row-group num_rows one high is red in DuckDB only. - clean: 14 of 14 on PG15 to PG19 in both twins. Ledger: 14 rows, 14 dated by the 11 mutations and 0 never. suites_not_covered 75 -> 74, cluster_tests 3222 -> 3236, guard_tests 804 -> 808, TESTS.md section 296. The ledger gate, guards (808), name grading and the self-test pass over 4adb2e7a. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 5 +- test/check_ledger.tsv | 14 + test/check_ledger_budget.txt | 7 +- test/parquet_export.sh | 61 +++- test/pytest/TESTS.md | 30 ++ test/pytest/expected_tests.txt | 8 +- test/pytest/pgc_vacuity.py | 5 +- test/pytest/requirements-test.txt | 1 + test/pytest/test_capability_decline.py | 60 ++++ test/pytest/test_compare_to_bash.py | 1 + test/pytest/test_parquet_export.py | 371 +++++++++++++++++++++++++ test/pytest/unported_suites.txt | 1 - 12 files changed, 547 insertions(+), 17 deletions(-) create mode 100644 test/pytest/test_parquet_export.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cf16dc30a..5ab335621 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -309,8 +309,11 @@ jobs: # developer box without it; in CI that would be green forever, so CI proves # the module imports and sets PGC_REQUIRE, which turns a decline into a # failure (OffgridwithJD, review of #1286). + # DUCKDB TOO (#1341): the one reader of an exported Parquet file that is not + # pyarrow, pinned beside it in requirements-test.txt and required the same way. /tmp/pgcvenv/bin/python -c 'import pyarrow, pyarrow.parquet; print("pyarrow", pyarrow.__version__)' - export PGC_REQUIRE=pyarrow + /tmp/pgcvenv/bin/python -c 'import duckdb; print("duckdb", duckdb.__version__)' + export PGC_REQUIRE=pyarrow,duckdb PYTHONPATH=. /tmp/pgcvenv/bin/pytest -q \ --pg-config /usr/lib/postgresql/${{ matrix.pg }}/bin/pg_config \ --pgc-expect-tests "$WANT" \ diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index 93e0c6266..acd76d81e 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -5622,6 +5622,20 @@ parquet_count_bounds parquet_count_bounds premise: the server is up before any m parquet_count_bounds parquet_count_bounds the num_rows rejection names num_rows in its detail 15;16;17;18;19 never - parquet_count_bounds parquet_count_bounds the num_values rejection names num_values in its detail 15;16;17;18;19 never - parquet_count_bounds parquet_count_bounds the server is still up after the honest import 15;16;17;18;19 never - +parquet_export parquet_export duckdb extended row count 15;16;17;18;19 2026-10-22 src/columnar_parquet.c: row-group num_rows written one high +parquet_export parquet_export duckdb non-null count(b) 15;16;17;18;19 2026-10-22 src/columnar_parquet.c: a negative int4 written as null;src/columnar_parquet.c: leaf names written without their first character;src/columnar_parquet.c: row-group num_rows written one high +parquet_export parquet_export duckdb reads decimal 15;16;17;18;19 2026-10-22 src/columnar_parquet.c: DECIMAL written little-endian;src/columnar_parquet.c: leaf names written without their first character +parquet_export parquet_export duckdb row count 15;16;17;18;19 2026-10-22 src/columnar_parquet.c: row-group num_rows written one high +parquet_export parquet_export empty export rows written 15;16;17;18;19 2026-10-22 src/columnar_parquet.c: export_parquet returns its row count one high +parquet_export parquet_export empty parquet readable 15;16;17;18;19 2026-10-22 src/columnar_parquet.c: leaf names written without their first character +parquet_export parquet_export export_parquet rows written 15;16;17;18;19 2026-10-22 src/columnar_parquet.c: export_parquet returns its row count one high +parquet_export parquet_export extended export rows written 15;16;17;18;19 2026-10-22 src/columnar_parquet.c: export_parquet returns its row count one high +parquet_export parquet_export extended parquet types 15;16;17;18;19 2026-10-22 src/columnar_parquet.c: leaf names written without their first character;src/columnar_parquet.c: numeric(p,s) never mapped to DECIMAL +parquet_export parquet_export extended values match heap oracle 15;16;17;18;19 2026-10-22 src/columnar_parquet.c: DECIMAL written little-endian;src/columnar_parquet.c: a date written one day late;src/columnar_parquet.c: leaf names written without their first character;src/columnar_parquet.c: numeric(p,s) never mapped to DECIMAL +parquet_export parquet_export parquet schema 15;16;17;18;19 2026-10-22 src/columnar_parquet.c: int2 written without its INT_16 annotation;src/columnar_parquet.c: leaf names written without their first character +parquet_export parquet_export parquet values match heap oracle 15;16;17;18;19 2026-10-22 src/columnar_parquet.c: a negative int4 written as null;src/columnar_parquet.c: every bool written inverted;src/columnar_parquet.c: leaf names written without their first character +parquet_export parquet_export reject non-columnar table 15;16;17;18;19 2026-10-22 src/columnar_parquet.c: the columnar-relation check skipped +parquet_export parquet_export reject unsupported type 15;16;17;18;19 2026-10-22 src/columnar_parquet.c: the unsupported-type refusal formats the typmod as a type parquet_export_stats parquet_export_stats a bigint predicate skips 15;16;17;18;19 2026-10-21 columnar_parquet.c: pqstats_reset leaves hasBounds set, so each later group's bounds start from the reset 0;columnar_parquet.c: write_statistics writes no min_value or max_value for any type;columnar_parquet_reader.c: col < const skips a group on max >= const instead of min >= const parquet_export_stats parquet_export_stats a cross-type constant skips as the matching one does, on the identical predicate 15;16;17;18;19 2026-10-22 columnar_parquet_reader.c: the exact-type refusal before #1712 restored, so a constant of another type skips nothing parquet_export_stats parquet_export_stats a date predicate skips 15;16;17;18;19 2026-10-21 columnar_parquet.c: leaf_value_representable admits date and timestamp infinities, so they reach the data and the bounds;columnar_parquet.c: pqstats_reset leaves hasBounds set, so each later group's bounds start from the reset 0;columnar_parquet.c: write_statistics writes no min_value or max_value for any type;columnar_parquet_reader.c: col < const skips a group on max >= const instead of min >= const diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index e332815a2..b83b26542 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -15,7 +15,7 @@ # # Adding a check to a suite that is already covered does not move it, which is # what makes it safe to bound. -suites_not_covered 27 +suites_not_covered 26 # # checks_never_observed_red -- A CENSUS. NOT a ceiling, and it must not become # one. @@ -7114,4 +7114,9 @@ suites_not_covered 27 # never 4544 + 4 = 4548 # dated 3464 + 3 = 3467 # 4548 + 3467 == 8015 +# +# ledger rows 8015 + 14 = 8029 +# never 4548 + 0 = 4548 +# dated 3467 + 14 = 3481 +# 4548 + 3481 == 8029 checks_never_observed_red 4548 diff --git a/test/parquet_export.sh b/test/parquet_export.sh index 5619714d2..116cbb482 100755 --- a/test/parquet_export.sh +++ b/test/parquet_export.sh @@ -7,7 +7,8 @@ # empty/unicode text, bytea) that mirrors a heap table, reads it back, and # asserts every value equals the heap oracle. pyarrow provides exact typed # values for the comparison; if the DuckDB CLI is present it is used as a second -# independent reader (row count and a sum). Also covers multiple row groups, an +# independent reader (row counts, a non-null count and a decimal), and without it +# those four checks are skipped by name. Also covers multiple row groups, an # empty table, and error cases. # # Requires pyarrow (python3-pyarrow); if it is not importable the suite skips @@ -27,13 +28,32 @@ if ! python3 -c 'import pyarrow.parquet' 2>/dev/null; then pgc_skip pyarrow "pyarrow not available; skipping Parquet export verification" fi -expect_error() { - local label="$1" sql="$2" - if psql_run "$sql" >/dev/null 2>&1; then - check "$label (expected error)" "succeeded" "error" +# expect_refusal NAME SQL MESSAGE -- SQL must fail with MESSAGE in its error. Any +# error used to count, so a backend crash or an unrelated failure passed as the +# refusal (#1341): with the columnar check removed, exporting a heap failed for +# another reason, and with the type check removed the backend aborted, and both +# passed. psql's exit status says whether it failed; the message says why. +expect_refusal() { + local name="$1" sql="$2" msg="$3" out got + if out="$(psql_run "$sql" 2>&1)"; then + got="succeeded" + elif grep -qF -- "$msg" <<<"$out"; then + got="refused: $msg" else - check "$label" "error" "error" + got="other error: $(head -n1 <<<"$out")" fi + check "$name" "$got" "refused: $msg" +} + +# duckdb_skip NAME -- without the duckdb CLI, record the DuckDB check NAME as +# skipped, by name. The four checks used to be left out without a word, so a run +# without DuckDB counted four checks fewer and said nothing (#1341). The twin +# declines the same four by the same names. A skip, not check_unrunnable: the +# suites job installs no DuckDB CLI, and an unrunnable check makes the suite +# INCOMPLETE, which fails its major there. CI holds the four in the twin instead, +# where PGC_REQUIRE names duckdb and turns a decline into a failure. +duckdb_skip() { + check_skip "$1" "SKIP $1 (the duckdb CLI is not installed)" "duckdb CLI not installed" } PARQUET="$PGC_WORKDIR/t.parquet" @@ -85,7 +105,13 @@ def feq(x,y): if math.isnan(x) and math.isnan(y): return True return x==y return x==y -ok = set(ar)==set(pg) and all(all(feq(a,b) for a,b in zip(ar[k],pg[k])) for k in pg) +# The ids the generator wrote, so that two empty readings cannot agree (#1341). +ids=set(range(1,80001))|{100001,100002,100003} +# And the file's own row count: rows are compared by id, so a row written twice +# would otherwise collapse into one. +if set(pg)!=ids or set(ar)!=ids or tbl.num_rows!=len(ids): + print("ROWS heap %d parquet %d, not the generator's 80003"%(len(pg),tbl.num_rows)); sys.exit(0) +ok = all(all(feq(a,b) for a,b in zip(ar[k],pg[k])) for k in pg) print("MATCH" if ok else "MISMATCH") PY )" @@ -104,8 +130,13 @@ if command -v duckdb >/dev/null 2>&1; then dcount="$(duckdb -noheader -list -c "SELECT count(*) FROM read_parquet('$PARQUET');" 2>/dev/null | tr -d '[:space:]')" check "duckdb row count" "$dcount" "80003" dsum_d="$(duckdb -noheader -list -c "SELECT count(b) FROM read_parquet('$PARQUET');" 2>/dev/null | tr -d '[:space:]')" - pcnt="$(q "SELECT count(b) FROM t_col;")" - check "duckdb non-null count(b)" "$dsum_d" "$pcnt" + # The generator's count, 80000 rows plus the two edge rows with a b: t_col is + # the table under test, so its own count agreed with whatever it lost, and with + # an empty read on both sides (#1341). + check_num "duckdb non-null count(b)" "$dsum_d" "80002" +else + duckdb_skip "duckdb row count" + duckdb_skip "duckdb non-null count(b)" fi # Empty table. @@ -122,10 +153,12 @@ check "empty parquet readable" "$er" "0 a,b" # Errors. echo "-- argument validation" -expect_error "reject non-columnar table" "SELECT pgcolumnar.export_parquet('t_heap', '$PGC_WORKDIR/x.parquet');" +expect_refusal "reject non-columnar table" "SELECT pgcolumnar.export_parquet('t_heap', '$PGC_WORKDIR/x.parquet');" \ + "is not a columnar table" psql_run "CREATE TABLE t_un (a int, p point) USING pgcolumnar;" psql_run "INSERT INTO t_un VALUES (1, '(1,2)');" -expect_error "reject unsupported type" "SELECT pgcolumnar.export_parquet('t_un', '$PGC_WORKDIR/x.parquet');" +expect_refusal "reject unsupported type" "SELECT pgcolumnar.export_parquet('t_un', '$PGC_WORKDIR/x.parquet');" \ + 'column "p" has type point, which columnar parquet export does not support' # --------------------------------------------------------------------------- # Extended type coverage: date, time, timestamp(+tz), uuid, numeric(p,s) as @@ -195,6 +228,9 @@ pg = {} for r in csv.reader(open(sys.argv[2],newline='')): pg[int(r[0])] = (int(r[0]), I(r[1]), I(r[2]), I(r[3]), I(r[4]), N(r[5]), N(r[6]), N(r[7]), N(r[8]), N(r[9])) +# The ids the INSERT wrote, so that two empty readings cannot agree (#1341). +if set(pg)!={1,2,3,4} or set(ar)!={1,2,3,4} or t.num_rows!=4: + print("ROWS heap %r parquet %r, not ids 1..4"%(sorted(pg),ids)); sys.exit(0) print("MATCH" if ar==pg else "MISMATCH %r %r"%(ar,pg)) PY )" @@ -216,6 +252,9 @@ if command -v duckdb >/dev/null 2>&1; then check "duckdb reads decimal" "$dnum" "12345.6789" dext="$(duckdb -noheader -list -c "SELECT count(*) FROM read_parquet('$EXT');" 2>/dev/null | tr -d '[:space:]')" check "duckdb extended row count" "$dext" "4" +else + duckdb_skip "duckdb reads decimal" + duckdb_skip "duckdb extended row count" fi pgc_summary diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 8a9b6c02d..ba7cdf003 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -390,6 +390,7 @@ behaviour, the source of that number is named. - [342. test_objstore_export_double_cancel.py: a second cancel during a remote export's cleanup keeps the export's own error](#342-test_objstore_export_double_cancelpy-a-second-cancel-during-a-remote-exports-cleanup-keeps-the-exports-own-error) - [343. test_parquet_timestamp_zone.py: a Parquet timestamp read into the other timestamp type follows PostgreSQL's casts](#343-test_parquet_timestamp_zonepy-a-parquet-timestamp-read-into-the-other-timestamp-type-follows-postgresqls-casts) - [344. test_parquet_skip_cross_type.py: Parquet row-group skipping uses a constant of another type in the operator family](#344-test_parquet_skip_cross_typepy-parquet-row-group-skipping-uses-a-constant-of-another-type-in-the-operator-family) +- [345. test_parquet_export.py: pgcolumnar.export_parquet writes a file two readers read back equal to the heap](#345-test_parquet_exportpy-pgcolumnarexport_parquet-writes-a-file-two-readers-read-back-equal-to-the-heap) ## 1. How to read a test in here @@ -6973,6 +6974,10 @@ Written red first: all seven failed before `pgc_vacuity` had the methods. The fo | `test_a_missing_dependency_of_pyarrow_fails_rather_than_declines` | a `pyarrow` that cannot import its own dependency raises `ModuleNotFoundError` naming that dependency, and fails rather than declines; shown red with `require_module`'s `.name` check deleted (OffgridwithJD, review of #1286) | | `test_every_unguarded_import_spelling_is_refused` | each of six spellings, `__import__` among them, is named and refused; a file using `require_module` is not | | `test_an_unguarded_import_is_refused_under_xdist_too` | under `-n 2` the controller names the unguarded import once and the run is refused, as the serial run is: workers ship their sites back through `workeroutput` (#1305) | +| `test_the_declinable_capabilities_are_exactly_pyarrow_and_duckdb` | the closed list is `pyarrow` and `duckdb`, and nothing else; `duckdb` joined on OffgridwithJD's decision on #1341 | +| `test_a_missing_duckdb_declines_by_name_and_the_run_stays_green` | a missing `duckdb` declines by name, with its capability, and the inner run exits 0 | +| `test_pgc_require_turns_a_duckdb_decline_into_a_failure` | with `PGC_REQUIRE=pyarrow,duckdb` a missing `duckdb` fails, and says duckdb is required | +| `test_an_unguarded_duckdb_import_is_refused` | a file that imports `duckdb` outside `require_module` is named and the run is refused | ## 92. test_tid_range_scan.py: a ctid range reads only the row groups that hold it @@ -12912,3 +12917,28 @@ The pytest twin of `test/parquet_skip_cross_type.sh` (#1712), every check by the | `test_pxt_a_timestamptz_equality_after_a_gap_finds_the_heap_row` | a timestamptz constant equal to a row's instant after a spring-forward gap finds that row, as the heap does | | `test_pxt_a_timestamp_constant_skips_one_gap_group` | control: a timestamp constant on the timestamp column skips 1 of the 2 row groups | | `test_pxt_the_timestamp_constant_returns_the_heap_rows` | and returns the rows the heap returns | + +## 345. test_parquet_export.py: pgcolumnar.export_parquet writes a file two readers read back equal to the heap + +The pytest twin of `test/parquet_export.sh` (ported for #1341), every check by the shell's name. A mixed-type columnar table of 80003 rows, more than one row group, is exported and read back by pyarrow and by DuckDB; an empty table, two refusals and the extended types (date, time, timestamps, uuid, numeric, json) follow. On a private database. + +### Every test + +Each value comparison needs the generator's ids on both sides and the file's row count, so two empty readings cannot agree. DuckDB's non-null count of b is held to the generator's 80002, not to the table under test. Each refusal must carry its own message. Without pyarrow every test declines. DuckDB is the `duckdb` Python package, pinned in `requirements-test.txt`, reached only through `require_module`; without it the four DuckDB tests decline by the shell's check names, as the shell skips them by name without the `duckdb` CLI. CI sets `PGC_REQUIRE=pyarrow,duckdb`, so there either decline fails. + +| test | what it holds | +| --- | --- | +| `test_export_parquet_reports_every_row` | export_parquet rows written | +| `test_parquet_values_match_the_heap_oracle` | parquet values match heap oracle | +| `test_the_parquet_schema_has_the_expected_types` | parquet schema | +| `test_duckdb_counts_every_parquet_row` | duckdb row count | +| `test_duckdb_counts_the_generators_non_null_b` | duckdb non-null count(b) | +| `test_an_empty_table_exports_no_parquet_rows` | empty export rows written | +| `test_an_empty_table_writes_a_readable_parquet_file` | empty parquet readable | +| `test_parquet_export_refuses_a_heap_table` | reject non-columnar table | +| `test_parquet_export_refuses_an_unsupported_type` | reject unsupported type | +| `test_the_extended_parquet_export_reports_every_row` | extended export rows written | +| `test_extended_parquet_values_match_the_heap_oracle` | extended values match heap oracle | +| `test_the_extended_parquet_types_are_the_mapped_ones` | extended parquet types | +| `test_duckdb_reads_the_parquet_decimal` | duckdb reads decimal | +| `test_duckdb_counts_every_extended_parquet_row` | duckdb extended row count | diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 9f27ceaf5..ecb450cc0 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -330,7 +330,9 @@ # test_runshape.py, by collection over NO_CLUSTER. # guard_tests 840 -> 842 for #1749's review: every literal fixture slot fits a worker's block, # and the corpus's highest slot gets a port, in test_runshape.py, by collection over NO_CLUSTER. -guard_tests 842 +# guard_tests 842 -> 846 for #1341's test_capability_decline.py: duckdb joins the declinable +# capabilities, held by four new tests, by collection over NO_CLUSTER. +guard_tests 846 # The complement: tests that need the driver and a throwaway cluster. Until #1016 these ran # in no CI job at all -- a quarter of the corpus, green when somebody ran them by hand and @@ -1222,4 +1224,6 @@ guard_tests 842 # (parametrized), by collection over the cluster files. # cluster_tests 4261 -> 4268 for #1712's seven spring-forward arms in test_parquet_skip_cross_type.py, # one per check, by collection over the cluster files. -cluster_tests 4268 +# cluster_tests 4268 -> 4282 for #1341's test_parquet_export.py, the twin of parquet_export.sh: +# 14 tests, one per check, by collection over the cluster files. +cluster_tests 4282 diff --git a/test/pytest/pgc_vacuity.py b/test/pytest/pgc_vacuity.py index cf2661ac3..0a383ec4b 100644 --- a/test/pytest/pgc_vacuity.py +++ b/test/pytest/pgc_vacuity.py @@ -242,7 +242,10 @@ def note_write(nodeid, cur): # through cannot_run and ends the run INCOMPLETE. Closed, so "optional" cannot grow # by one call site at a time. PGC_REQUIRE (comma-separated) names capabilities that # must be present here, as CI does, and turns a decline of them into a failure. -DECLINABLE_CAPABILITIES = ("pyarrow",) +# duckdb joined on the owner's decision on #1341 (2026-10-21): the one reader of an +# exported Parquet file that is not pyarrow, pinned in requirements-test.txt and +# required in CI by PGC_REQUIRE. +DECLINABLE_CAPABILITIES = ("pyarrow", "duckdb") class VacuityError(AssertionError): diff --git a/test/pytest/requirements-test.txt b/test/pytest/requirements-test.txt index 147de622f..bf3654eb2 100644 --- a/test/pytest/requirements-test.txt +++ b/test/pytest/requirements-test.txt @@ -8,3 +8,4 @@ pytest==9.1.1 pytest-xdist==3.8.0 psycopg[binary]==3.3.5 pyarrow==23.0.1 +duckdb==1.5.6 diff --git a/test/pytest/test_capability_decline.py b/test/pytest/test_capability_decline.py index d9f75348e..d0e632291 100644 --- a/test/pytest/test_capability_decline.py +++ b/test/pytest/test_capability_decline.py @@ -16,6 +16,10 @@ import spelling, each proven seen 5 the run's accounting line carries the declines +duckdb declines the same way, and is held to the same three of those rules here +(by name, PGC_REQUIRE, the import scan): it joined the closed list on the owner's +decision on #1341 (2026-10-21), as the second reader of an exported Parquet file. + Runs pytest inside pytest; needs no cluster and no driver. """ @@ -57,6 +61,62 @@ def test_inner_required_fails(expect, monkeypatch): "and it fails BECAUSE pyarrow is required, not for another reason") +def test_the_declinable_capabilities_are_exactly_pyarrow_and_duckdb(expect): + # The list is closed, so "optional" cannot grow by one call site at a time. duckdb + # joined on OffgridwithJD's decision on #1341 (2026-10-21): the second reader of an + # exported Parquet file, pinned and required in CI like pyarrow. + import pgc_vacuity + expect.text(",".join(pgc_vacuity.DECLINABLE_CAPABILITIES), "pyarrow,duckdb", + "the declinable capabilities are pyarrow and duckdb, and nothing else") + + +def test_a_missing_duckdb_declines_by_name_and_the_run_stays_green(pytester, monkeypatch, expect): + # The caller's PGC_REQUIRE must not decide this test (CI exports it). + monkeypatch.delenv("PGC_REQUIRE", raising=False) + pytester.makepyfile(ABSENT + """ + def test_inner_duckdb_declines_green(expect, monkeypatch): + monkeypatch.setitem(sys.modules, "duckdb", None) + db = expect.require_module("duckdb", name="the second reader counts the rows") + assert db is None +""") + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "a test declined on duckdb leaves the run green", passed=1, failed=0) + expect.num(result.ret, 0, "and the run with a duckdb decline exits 0") + expect.contains(result.stdout.str(), + "DECLINED test_a_missing_duckdb_declines_by_name_and_the_run_stays_green.py::test_inner_duckdb_declines_green: duckdb", + "and the duckdb decline is named, with its capability") + + +def test_pgc_require_turns_a_duckdb_decline_into_a_failure(pytester, monkeypatch, expect): + monkeypatch.setenv("PGC_REQUIRE", "pyarrow,duckdb") + pytester.makepyfile(ABSENT + """ + def test_inner_duckdb_required_fails(expect, monkeypatch): + monkeypatch.setitem(sys.modules, "duckdb", None) + expect.require_module("duckdb", name="the second reader counts the rows") +""") + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "with PGC_REQUIRE=pyarrow,duckdb a missing duckdb fails", + passed=0, failed=1) + expect.contains(result.stdout.str(), "PGC_REQUIRE names duckdb", + "and it fails BECAUSE duckdb is required, not for another reason") + + +def test_an_unguarded_duckdb_import_is_refused(pytester, monkeypatch, expect): + # The caller's PGC_REQUIRE must not decide this test (CI exports it). + monkeypatch.delenv("PGC_REQUIRE", raising=False) + # Inside a function that never runs: the scan reads the AST, so it must find the + # import there too, and the real duckdb is never imported. An in-process import + # that pytester's sys.modules snapshot then removed left duckdb's C extension + # unimportable for every later test in the session (measured: the four DuckDB + # tests of test_parquet_export.py failed after this file, in one session). + pytester.makepyfile(test_duck_bare="def _never_called():\n import duckdb\n\n\n" + "def test_x(expect):\n expect.num(1, 1, 'x')\n") + result = pytester.runpytest("-p", "pgc_vacuity") + expect.contains(result.stdout.str(), "test_duck_bare.py imports duckdb outside require_module", + "a file importing duckdb outside require_module is named") + expect.num(1 if result.ret != 0 else 0, 1, "and a run carrying it is refused") + + def test_a_decline_after_a_counted_check_is_refused(pytester, monkeypatch, expect): # The caller's PGC_REQUIRE must not decide this test (CI exports it). monkeypatch.delenv("PGC_REQUIRE", raising=False) diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index 4093a5795..af3071a09 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -97,6 +97,7 @@ "parallel", "parallel_copy", "parallel_copy_dedup", + "parquet_export", "parquet_export_stats", "parallel_flush_optin", "parallel_degree", diff --git a/test/pytest/test_parquet_export.py b/test/pytest/test_parquet_export.py new file mode 100644 index 000000000..08134bdc5 --- /dev/null +++ b/test/pytest/test_parquet_export.py @@ -0,0 +1,371 @@ +"""pgcolumnar.export_parquet writes a Parquet file that pyarrow, and DuckDB where it is +installed, read back equal to a heap oracle (gap 27, piece 2). The pytest twin of +`test/parquet_export.sh`, every check by the shell's name (#1341). + +A mixed-type columnar table (NULLs, integer boundaries, NaN and infinities, empty and +unicode text, bytea) copied from a heap table is exported across more than one row +group (80003 rows, over PARQUET_ROWGROUP_ROWS of 65536), and every value must equal the +heap's; DuckDB, a second and independent reader, must count the same rows and the same +non-null b. An empty table writes a readable schema-only file; a heap table and an +unsupported column type are refused, each by its own message; and date, time, +timestamps, uuid, numeric and json export to the representation each maps to. + +ON A PRIVATE DATABASE, built once by a module fixture in the shell's order; each test +asserts one reading, so one red cannot stop the next (#1339). The heap oracle is read +over the driver with the shell's own SQL expressions, not through the file. Each file +reading requires the oracle and the file to hold the generator's ids, so two empty +sides cannot agree, and the file's row count to equal them: rows are compared by id, +so without it a row written twice would collapse into one. The shell skips whole +without pyarrow, so every test declines by the shell's skip name first. The shell +reads the file with the `duckdb` CLI where it is installed and skips those four checks +by name where it is not; here DuckDB is the `duckdb` Python package, pinned in +requirements-test.txt, and without it the same four tests decline by the same names. +CI sets PGC_REQUIRE=pyarrow,duckdb, which turns either decline into a failure. The +files live in `server_dir`, because the backend writes them. A failed query is a +`query_error()`. +""" +import math + +import psycopg +import pytest + +from pgc_vacuity import query_error + +DB = "pgc_parquet_export" +COLS = ["id", "a", "b", "c", "d", "e", "f", "g", "h"] +DEFS = "id int, a int2, b int4, c int8, d float4, e float8, f bool, g text, h bytea" +# > PARQUET_ROWGROUP_ROWS (65536) rows to force multiple row groups, plus explicit +# edge-case rows. +BODY = r""" + SELECT s AS id, (s%30000-15000)::int2, s*2, s::int8*1000, + (s::float4/3), (s::float8/7), (s%2=0), 'r'||s, + decode(lpad(to_hex(s),6,'0'),'hex') + FROM generate_series(1,80000) s + UNION ALL VALUES + (100001, NULL::int2, NULL::int4, NULL::int8, NULL::float4, NULL::float8, NULL::bool, NULL::text, NULL::bytea), + (100002, (-32768)::int2, (-2147483648)::int4, (-9223372036854775808)::int8, '-Infinity'::float4, 'NaN'::float8, false, ''::text, '\x'::bytea), + (100003, (32767)::int2, (2147483647)::int4, (9223372036854775807)::int8, 'Infinity'::float4, (3.14)::float8, true, 'unicode q'::text, '\x00ff10'::bytea) +""" +# The ids the generator wrote. +IDS = set(range(1, 80001)) | {100001, 100002, 100003} +# float4 widened to float8 so its value matches Parquet's, as the shell's COPY does +ORACLE = "SELECT id, a, b, c, (d::float8), e, f, g, encode(h,'hex') FROM t_heap ORDER BY id" +EXT_INSERT = """INSERT INTO t_ext_heap VALUES + (1,'2021-03-04','12:34:56.789012','2021-03-04 12:34:56.789012', + '2021-03-04 12:34:56.789012+00','11111111-2222-3333-4444-555555555555', + 12345.6789, 9999999999999999.1234, '{"a": 1}', '{"b": 2}'), + (2,'1970-01-01','00:00:00','2000-01-01 00:00:00','2000-01-01 00:00:00+00', + '00000000-0000-0000-0000-000000000000', -0.0001, -1234567890.5, + '[1, 2, 3]', '[3, 2, 1]'), + (3, NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL), + (4,'infinity','23:59:59.999999','infinity','-infinity', + 'ffffffff-ffff-ffff-ffff-ffffffffffff', 'NaN', 'NaN', 'null', 'true')""" +EXT_ORACLE = """SELECT id, + CASE WHEN dt IS NULL OR NOT isfinite(dt) THEN NULL ELSE (dt - DATE '1970-01-01') END, + CASE WHEN tm IS NULL THEN NULL ELSE (extract(epoch from tm)*1000000)::bigint END, + CASE WHEN ts IS NULL OR NOT isfinite(ts) THEN NULL ELSE (extract(epoch from ts)*1000000)::bigint END, + CASE WHEN tz IS NULL OR NOT isfinite(tz) THEN NULL ELSE (extract(epoch from tz)*1000000)::bigint END, + CASE WHEN u IS NULL THEN NULL ELSE replace(u::text,'-','') END, + CASE WHEN num IS NULL OR num = 'NaN'::numeric THEN NULL ELSE num::text END, + CASE WHEN numun IS NULL THEN NULL ELSE numun::text END, + j::text, jb::text + FROM t_ext_heap ORDER BY id""" +SCHEMA = "id:int32,a:int16,b:int32,c:int64,d:float,e:double,f:bool,g:string,h:binary" +EXT_TYPES = "date32[day]|time64[us]|fixed_size_binary[16]|decimal128(20, 4)" +NOT_COLUMNAR = "is not a columnar table" +UNSUPPORTED = 'column "p" has type point, which columnar parquet export does not support' + + +def _failed(exc): + return query_error(str(exc).splitlines()[0] if str(exc) else type(exc).__name__) + + +def _value(dsn, sql): + """-> the first column of the first row as text, or the failure as a query_error().""" + try: + with psycopg.connect(dsn, autocommit=True) as conn: + row = conn.execute(sql).fetchone() + except psycopg.Error as exc: + return _failed(exc) + return "" if row is None or row[0] is None else str(row[0]) + + +def _rows(dsn, sql): + """-> every row as a tuple, or the failure as a query_error().""" + try: + with psycopg.connect(dsn, autocommit=True) as conn: + return [tuple(r) for r in conn.execute(sql)] + except psycopg.Error as exc: + return _failed(exc) + + +def _run(dsn, sql): + """Run one statement and swallow its failure, as the shell's psql_run does: a + failed step shows up in the readings after it.""" + try: + with psycopg.connect(dsn, autocommit=True) as conn: + conn.execute(sql) + except psycopg.Error: + pass + + +def _refusal(dsn, sql, msg): + """-> "refused: MSG" when `sql` fails with `msg`, "succeeded", or the other error. + Any error used to count, so a backend crash passed as the refusal.""" + try: + with psycopg.connect(dsn, autocommit=True) as conn: + conn.execute(sql) + except psycopg.Error as exc: + text = str(exc) + return f"refused: {msg}" if msg in text else f"other error: {text.splitlines()[0] if text else type(exc).__name__}" + return "succeeded" + + +@pytest.fixture(scope="module") +def pqx(pgc_cluster, server_dir): + with psycopg.connect(pgc_cluster.dsn(), autocommit=True) as a: + a.execute(f"DROP DATABASE IF EXISTS {DB} WITH (FORCE)") + a.execute(f"CREATE DATABASE {DB}") + try: + dsn = pgc_cluster.dsn(dbname=DB) + f = {k: server_dir / n for k, n in + (("t", "t.parquet"), ("e", "e.parquet"), ("x", "x.parquet"), ("ext", "ext.parquet"))} + _run(dsn, "CREATE EXTENSION IF NOT EXISTS pgcolumnar") + # make_pair and load_pair: generated once into heap, then copied to columnar + _run(dsn, f"CREATE TABLE t_heap ({DEFS}) USING heap") + _run(dsn, f"CREATE TABLE t_col ({DEFS}) USING pgcolumnar") + _run(dsn, f"INSERT INTO t_heap {BODY}") + _run(dsn, "INSERT INTO t_col SELECT * FROM t_heap") + r = {"files": f, "rows": _value(dsn, f"SELECT pgcolumnar.export_parquet('t_col', '{f['t']}')"), + "oracle": _rows(dsn, ORACLE)} + _run(dsn, "CREATE TABLE t_empty (a int, b text) USING pgcolumnar") + r["empty rows"] = _value(dsn, f"SELECT pgcolumnar.export_parquet('t_empty', '{f['e']}')") + r["non-columnar"] = _refusal(dsn, f"SELECT pgcolumnar.export_parquet('t_heap', '{f['x']}')", + NOT_COLUMNAR) + _run(dsn, "CREATE TABLE t_un (a int, p point) USING pgcolumnar") + _run(dsn, "INSERT INTO t_un VALUES (1, '(1,2)')") + r["unsupported"] = _refusal(dsn, f"SELECT pgcolumnar.export_parquet('t_un', '{f['x']}')", + UNSUPPORTED) + _run(dsn, "CREATE TABLE t_ext_heap (id int, dt date, tm time, ts timestamp, " + "tz timestamptz, u uuid, num numeric(20,4), numun numeric, j json, jb jsonb)") + _run(dsn, "CREATE TABLE t_ext_col (LIKE t_ext_heap) USING pgcolumnar") + _run(dsn, EXT_INSERT) + _run(dsn, "INSERT INTO t_ext_col SELECT * FROM t_ext_heap") + r["ext rows"] = _value(dsn, f"SELECT pgcolumnar.export_parquet('t_ext_col', '{f['ext']}')") + r["ext oracle"] = _rows(dsn, EXT_ORACLE) + yield r + finally: + with psycopg.connect(pgc_cluster.dsn(), autocommit=True) as a: + a.execute(f"DROP DATABASE IF EXISTS {DB} WITH (FORCE)") + + +def _read(pa, path): + # pyarrow.parquet is a submodule: each caller has imported it through + # require_module, which binds it on the package + return pa.parquet.read_table(str(path)) + + +def _reading(fn, pa, *args): + """-> fn(pa, *args), or the failure to read the file as a query_error().""" + try: + return fn(pa, *args) + except (OSError, KeyError, IndexError, ValueError, TypeError, pa.ArrowException) as exc: + # a missing or unreadable file, or a field it lacks, is a failed reading + return query_error(f"{type(exc).__name__}: {exc}") + + +def _feq(x, y): + if isinstance(x, float) or isinstance(y, float): + if x is None or y is None: + return x is y + if math.isnan(x) and math.isnan(y): + return True + return x == y + return x == y + + +def _values(pa, path, oracle): + if isinstance(oracle, str): + return oracle + tbl = _read(pa, path) + if tbl.schema.names != COLS: + return "SCHEMA_NAMES_MISMATCH" + ad = tbl.to_pydict() + ar = {ad["id"][i]: tuple(ad[c][i] for c in COLS) for i in range(tbl.num_rows)} + pg = {r[0]: (r[0], r[1], r[2], r[3], r[4], r[5], r[6], r[7], + None if r[8] is None else bytes.fromhex(r[8])) for r in oracle} + # the generator's ids on both sides, so that two empty readings cannot agree, and + # the file's own count, so that a row written twice cannot collapse into one + if set(pg) != IDS or set(ar) != IDS or tbl.num_rows != len(IDS): + return f"ROWS heap {len(pg)} parquet {tbl.num_rows}, not the generator's 80003" + ok = all(all(_feq(a, b) for a, b in zip(ar[k], pg[k])) for k in pg) + return "MATCH" if ok else "MISMATCH" + + +def _schema(pa, path): + return ",".join(f"{f.name}:{f.type}" for f in _read(pa, path).schema) + + +def _empty(pa, path): + t = _read(pa, path) + return f"{t.num_rows} {','.join(t.schema.names)}" + + +def _ext_values(pa, path, oracle): + if isinstance(oracle, str): + return oracle + t = _read(pa, path) + + def col(n): + return t.column(n) + + def i(n): + return col(n).cast(pa.int64()).to_pylist() + + ids = col("id").to_pylist() + dt = col("dt").cast(pa.int32()).to_pylist() # date32 casts to int32, not int64 + tm, ts, tz = i("tm"), i("ts"), i("tz") + u = [None if v is None else v.hex() for v in col("u").to_pylist()] + num = [None if v is None else str(v) for v in col("num").to_pylist()] + nun = [None if v is None else str(v) for v in col("numun").to_pylist()] + j, jb = col("j").to_pylist(), col("jb").to_pylist() + ar = {ids[k]: (ids[k], dt[k], tm[k], ts[k], tz[k], u[k], num[k], nun[k], j[k], jb[k]) + for k in range(t.num_rows)} + pg = {r[0]: tuple(r) for r in oracle} + if set(pg) != {1, 2, 3, 4} or set(ar) != {1, 2, 3, 4} or t.num_rows != 4: + return f"ROWS heap {sorted(pg)!r} parquet {ids!r}, not ids 1..4" + return "MATCH" if ar == pg else f"MISMATCH {ar!r} {pg!r}" + + +def _ext_types(pa, path): + d = {f.name: str(f.type) for f in _read(pa, path).schema} + return f"{d['dt']}|{d['tm']}|{d['u']}|{d['num']}" + + +def _duckdb_value(db, sql): + """-> the one value DuckDB answers `sql` with, or its failure as a query_error(). + + `db` is the duckdb module that the test's require_module returned: a fixture or a + module-level import could not decline by name. A connection of its own, in memory, + so nothing outlives the reading.""" + try: + con = db.connect() + try: + row = con.execute(sql).fetchone() + finally: + con.close() + except db.Error as exc: + return query_error(f"duckdb: {exc}") + if row is None or row[0] is None: + return query_error(f"duckdb answered no value to {sql!r}") + return row[0] + + +def test_export_parquet_reports_every_row(pqx, expect): + if expect.require_module("pyarrow", name="pyarrow not available; skipping Parquet export verification") is None: + return + expect.text(pqx["rows"], "80003", "export_parquet rows written") + + +def test_parquet_values_match_the_heap_oracle(pqx, expect): + pa = expect.require_module("pyarrow", name="pyarrow not available; skipping Parquet export verification") + if pa is None or expect.require_module("pyarrow.parquet", name="pyarrow not available; skipping Parquet export verification") is None: + return + expect.text(_reading(_values, pa, pqx["files"]["t"], pqx["oracle"]), "MATCH", + "parquet values match heap oracle") + + +def test_the_parquet_schema_has_the_expected_types(pqx, expect): + pa = expect.require_module("pyarrow", name="pyarrow not available; skipping Parquet export verification") + if pa is None or expect.require_module("pyarrow.parquet", name="pyarrow not available; skipping Parquet export verification") is None: + return + expect.text(_reading(_schema, pa, pqx["files"]["t"]), SCHEMA, "parquet schema") + + +def test_duckdb_counts_every_parquet_row(pqx, expect): + if expect.require_module("pyarrow", name="pyarrow not available; skipping Parquet export verification") is None: + return + db = expect.require_module("duckdb", name="duckdb row count") + if db is None: + return + expect.num(_duckdb_value(db, f"SELECT count(*) FROM read_parquet('{pqx['files']['t']}')"), 80003, + "duckdb row count") + + +def test_duckdb_counts_the_generators_non_null_b(pqx, expect): + if expect.require_module("pyarrow", name="pyarrow not available; skipping Parquet export verification") is None: + return + db = expect.require_module("duckdb", name="duckdb non-null count(b)") + if db is None: + return + # 80000 generated rows plus the two edge rows with a b, from the generator: t_col is + # the table under test, so its own count agreed with whatever it lost (#1341) + expect.num(_duckdb_value(db, f"SELECT count(b) FROM read_parquet('{pqx['files']['t']}')"), 80002, + "duckdb non-null count(b)") + + +def test_an_empty_table_exports_no_parquet_rows(pqx, expect): + if expect.require_module("pyarrow", name="pyarrow not available; skipping Parquet export verification") is None: + return + expect.text(pqx["empty rows"], "0", "empty export rows written") + + +def test_an_empty_table_writes_a_readable_parquet_file(pqx, expect): + pa = expect.require_module("pyarrow", name="pyarrow not available; skipping Parquet export verification") + if pa is None or expect.require_module("pyarrow.parquet", name="pyarrow not available; skipping Parquet export verification") is None: + return + expect.text(_reading(_empty, pa, pqx["files"]["e"]), "0 a,b", "empty parquet readable") + + +def test_parquet_export_refuses_a_heap_table(pqx, expect): + if expect.require_module("pyarrow", name="pyarrow not available; skipping Parquet export verification") is None: + return + expect.text(pqx["non-columnar"], f"refused: {NOT_COLUMNAR}", "reject non-columnar table") + + +def test_parquet_export_refuses_an_unsupported_type(pqx, expect): + if expect.require_module("pyarrow", name="pyarrow not available; skipping Parquet export verification") is None: + return + expect.text(pqx["unsupported"], f"refused: {UNSUPPORTED}", "reject unsupported type") + + +def test_the_extended_parquet_export_reports_every_row(pqx, expect): + if expect.require_module("pyarrow", name="pyarrow not available; skipping Parquet export verification") is None: + return + expect.text(pqx["ext rows"], "4", "extended export rows written") + + +def test_extended_parquet_values_match_the_heap_oracle(pqx, expect): + pa = expect.require_module("pyarrow", name="pyarrow not available; skipping Parquet export verification") + if pa is None or expect.require_module("pyarrow.parquet", name="pyarrow not available; skipping Parquet export verification") is None: + return + expect.text(_reading(_ext_values, pa, pqx["files"]["ext"], pqx["ext oracle"]), "MATCH", + "extended values match heap oracle") + + +def test_the_extended_parquet_types_are_the_mapped_ones(pqx, expect): + pa = expect.require_module("pyarrow", name="pyarrow not available; skipping Parquet export verification") + if pa is None or expect.require_module("pyarrow.parquet", name="pyarrow not available; skipping Parquet export verification") is None: + return + expect.text(_reading(_ext_types, pa, pqx["files"]["ext"]), EXT_TYPES, "extended parquet types") + + +def test_duckdb_reads_the_parquet_decimal(pqx, expect): + if expect.require_module("pyarrow", name="pyarrow not available; skipping Parquet export verification") is None: + return + db = expect.require_module("duckdb", name="duckdb reads decimal") + if db is None: + return + # cast in DuckDB, so the reading is text as the CLI printed it + expect.text(_duckdb_value(db, f"SELECT num::VARCHAR FROM read_parquet('{pqx['files']['ext']}') WHERE id=1"), + "12345.6789", "duckdb reads decimal") + + +def test_duckdb_counts_every_extended_parquet_row(pqx, expect): + if expect.require_module("pyarrow", name="pyarrow not available; skipping Parquet export verification") is None: + return + db = expect.require_module("duckdb", name="duckdb extended row count") + if db is None: + return + expect.num(_duckdb_value(db, f"SELECT count(*) FROM read_parquet('{pqx['files']['ext']}')"), 4, + "duckdb extended row count") diff --git a/test/pytest/unported_suites.txt b/test/pytest/unported_suites.txt index 0c12ce47f..5d912b2f8 100644 --- a/test/pytest/unported_suites.txt +++ b/test/pytest/unported_suites.txt @@ -6,5 +6,4 @@ # in that file. A port PR removes its own line, so two ports never conflict here. # # The parity goal (#432) is this file empty. -parquet_export planner_choice_quality From bb1b864b4dec28491438e2675389a7c29ab3505c Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 22 Oct 2026 18:09:07 +0000 Subject: [PATCH 6/7] test: planner_choice_quality gains its pytest twin, and a plan that fails no longer reads as a fast one (#1341) Ports test/planner_choice_quality.sh to test/pytest/test_planner_choice_quality.py, all 11 checks by the shell's name, one test per check, on a private database with serial planning as the shell's cluster runs. Each arm's EXPLAIN, warming run and timed run share one session, so the plan named is the plan timed. Two holes in the shell, each measured on PG17 against main and fixed in both twins: - psql prints "Time:" after a statement that errored, and run_plan only looked for a timeout. So a chosen plan that raised an error was read as a fast plan. With PgColumnarBeginCustomScan raising at execution, or PgColumnarBeginGroupAggScan, main's shell PASSED every check (the grouped query "chose Custom Scan=1.911ms", 0.01x). run_plan now reads psql's exit status under ON_ERROR_STOP, and tells the statement-timeout cancel apart by its own message. A new check, "[$label] the chosen plan ran to completion", fails by name; a forced alternative that fails fails the ratio check as "nothing to compare" instead of standing in as the fastest plan (main read it as 15.94x over the bound). - The ratio check's name carried the bound and the node that won the comparison, so it was "... alternative (Index Scan)" when green and "... alternative (Custom Scan (PgColumnarScan))" when red: a red no green run had named. The names now carry only the query's label, and the values are in the display line. "forcing produced a different plan" loses its premise prefix (#1339): with pgcolumnar.enable_custom_scan ignored by the planner, it is the check that goes red. PGC_SKIP_TIMING: CI's suites job sets it, and run_all_versions.sh does not run the suite at all under it (is_timing_suite), which is why CI reports it skipped. Run directly under the flag, the shell builds 20,000 rows, runs the plan-shape checks and records the two measured checks per query as SKIP. The twin reads the same flag: it builds the same fixture, and declares the same two checks per query unrunnable under the shell's names, so the session ends INCOMPLETE (67), never green. Five mutations on PG17, each reddening the same names in shell and twin: scan100x (refined scan cost x100, the planner picks the index scan; 2 ratio checks, 23.3x-25.4x over three shell runs and 23.6x-36.7x over three twin runs), scanerrors 2, fetcherrors 2, gucignored 2, groupaggerrors 2. Clean, the ratio is 0.02x-0.05x on every run. 11/11 in shell and twin on each of 15-19. Ledger: 11 rows, 6 dated by the 5 mutations and 5 never. suites_not_covered 75 -> 74, cluster_tests 3222 -> 3233, TESTS.md section 296. The ledger gate, guards (804), name grading and the self-test pass over 4adb2e7a. Co-Authored-By: Claude Opus 5.5 --- test/check_ledger.tsv | 11 + test/check_ledger_budget.txt | 24 +- test/planner_choice_quality.sh | 102 ++++--- test/pytest/TESTS.md | 23 ++ test/pytest/expected_tests.txt | 4 +- test/pytest/test_compare_to_bash.py | 1 + test/pytest/test_planner_choice_quality.py | 294 +++++++++++++++++++++ test/pytest/unported_suites.txt | 1 - 8 files changed, 425 insertions(+), 35 deletions(-) create mode 100644 test/pytest/test_planner_choice_quality.py diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index acd76d81e..7d0187e8c 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -6164,6 +6164,17 @@ phase6 phase6 sum(id) 15;16;17;18;19 never - phase6 phase6 sum(numeric) 15;16;17;18;19 2026-10-22 columnar_encoding.c: an uncompressed varlena decode turns the first 'w' byte of each vector into 'W' phase6 phase6 sum(smallint) 15;16;17;18;19 never - phase6 phase6 vectors formed 15;16;17;18;19 never - +planner_choice_quality planner_choice_quality [range scan with aggregate] forcing produced a different plan 15;16;17;18;19 2026-10-22 src/columnar_customscan.c: the planner ignores pgcolumnar.enable_custom_scan=off +planner_choice_quality planner_choice_quality [range scan with aggregate] the chosen plan is within the bound of the best alternative 15;16;17;18;19 2026-10-22 src/columnar_customscan.c: pgcolumnar_refined_scan_cost prices the custom scan 100x, so the planner picks the index scan;src/columnar_tableam.c: pgcolumnar_index_fetch_tuple raises an error on every fetch;src/columnar_vector.c: PgColumnarBeginGroupAggScan raises an error whenever the grouped aggregate executes +planner_choice_quality planner_choice_quality [range scan with aggregate] the chosen plan ran to completion 15;16;17;18;19 2026-10-22 src/columnar_vector.c: PgColumnarBeginGroupAggScan raises an error whenever the grouped aggregate executes +planner_choice_quality planner_choice_quality [wide range, narrow projection] forcing produced a different plan 15;16;17;18;19 2026-10-22 src/columnar_customscan.c: the planner ignores pgcolumnar.enable_custom_scan=off +planner_choice_quality planner_choice_quality [wide range, narrow projection] the chosen plan is within the bound of the best alternative 15;16;17;18;19 2026-10-22 src/columnar_customscan.c: PgColumnarBeginCustomScan raises an error whenever the scan executes;src/columnar_customscan.c: pgcolumnar_refined_scan_cost prices the custom scan 100x, so the planner picks the index scan;src/columnar_tableam.c: pgcolumnar_index_fetch_tuple raises an error on every fetch +planner_choice_quality planner_choice_quality [wide range, narrow projection] the chosen plan ran to completion 15;16;17;18;19 2026-10-22 src/columnar_customscan.c: PgColumnarBeginCustomScan raises an error whenever the scan executes +planner_choice_quality planner_choice_quality fixture rows 15;16;17;18;19 never - +planner_choice_quality planner_choice_quality premise: [range scan with aggregate] the chosen plan was identified 15;16;17;18;19 never - +planner_choice_quality planner_choice_quality premise: [wide range, narrow projection] the chosen plan was identified 15;16;17;18;19 never - +planner_choice_quality planner_choice_quality premise: the index key is correlated, which is the case that misprices 15;16;17;18;19 never - +planner_choice_quality planner_choice_quality premise: the payload did not compress, so the fetch path is exercised 15;16;17;18;19 never - preimage_rewrite preimage_rewrite ...and prunes as well as the equivalent explicit range 15;16;17;18;19 2026-10-21 columnar_customscan.c: the date_trunc preimage keys are derived and then dropped, so no rewrite happens preimage_rewrite preimage_rewrite ...and still answers correctly with the fold enabled 15;16;17;18;19 never - preimage_rewrite preimage_rewrite ...and the heap agrees it is unsatisfiable 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index b83b26542..95a53e57b 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -15,7 +15,7 @@ # # Adding a check to a suite that is already covered does not move it, which is # what makes it safe to bound. -suites_not_covered 26 +suites_not_covered 25 # # checks_never_observed_red -- A CENSUS. NOT a ceiling, and it must not become # one. @@ -7119,4 +7119,24 @@ suites_not_covered 26 # never 4548 + 0 = 4548 # dated 3467 + 14 = 3481 # 4548 + 3481 == 8029 -checks_never_observed_red 4548 +# +# #1341, planner_choice_quality seeded, with its twin (jdatcmd): 4548 -> 4553, and +# suites_not_covered 26 -> 25. The suite had no rows. It enters with 11 checks from +# five-major runs (all green on each of 15-19, shell and twin alike), after two fixes. +# psql prints "Time:" after a statement that errored, so a chosen plan that raised an +# error in 2 ms was read as a plan that answered in 2 ms: with the custom scan or the +# grouped aggregate raising at execution, main's shell passed every check. The chosen +# plan must now run to completion, by psql's exit status, and a forced alternative that +# fails fails the comparison. And the ratio check's name carried the node that won, so +# it had one name green and another red; the names now carry only the query's label. +# "forcing produced a different plan" lost its premise prefix (#1339): with +# pgcolumnar.enable_custom_scan ignored it is the check that goes red. 6 are dated by +# five mutations, each reddening the same names in both twins. 5 arrive never: fixture +# rows and four premises. The suite stays out of CI's suites job under PGC_SKIP_TIMING; +# its twin declares the two measured checks per query unrunnable under that flag. +# +# ledger rows 8029 + 11 = 8040 +# never 4548 + 5 = 4553 +# dated 3481 + 6 = 3487 +# 4553 + 3487 == 8040 +checks_never_observed_red 4553 diff --git a/test/planner_choice_quality.sh b/test/planner_choice_quality.sh index 7e8ccd1a7..2c1342f81 100755 --- a/test/planner_choice_quality.sh +++ b/test/planner_choice_quality.sh @@ -91,10 +91,18 @@ check "premise: the index key is correlated, which is the case that misprices" \ FROM pg_stats WHERE tablename='pq' AND attname='k'")" "yes" # Time a query under a given setting, and report which scan node ran. -run_plan() { # run_plan -> " " - local node ms out +# +# A FAILED RUN IS NOT A FAST ONE. psql prints "Time:" after a statement that +# errored as well as after one that succeeded, so this used to read a plan that +# raised an error in 2 ms as a plan that answered in 2 ms, and the ratio below +# passed with the chosen plan unable to run at all (measured: a custom scan that +# raises at execution left the suite PASSED). psql's exit status under +# ON_ERROR_STOP is the signal now, and the statement-timeout cancel is told +# apart by its own message rather than by any line containing "timeout". +run_plan() { # run_plan -> "\t" + local node ms out rc node=$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ - -d "$PGC_DB" -Atq -c "$1" -c "EXPLAIN (COSTS OFF) $2" 2>&1 | + -d "$PGC_DB" -v ON_ERROR_STOP=1 -Atq -c "$1" -c "EXPLAIN (COSTS OFF) $2" 2>&1 | grep -oE 'Index Only Scan|Index Scan|Bitmap Heap Scan|Custom Scan \([A-Za-z]+\)|Seq Scan' | head -1) # No execution under PGC_SKIP_TIMING. The plan name above comes from EXPLAIN, # so every premise still holds; only the wall clock is unavailable. @@ -107,11 +115,18 @@ run_plan() { # run_plan -> " " # warm against a 3x bound -- the ordering alone could push a healthy ratio # over it. The discarded run also populates shared buffers for the timed one. env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ - -d "$PGC_DB" -Atq -c "SET statement_timeout='${PLAN_TIMEOUT}s';" -c "$1" -c "$2" >/dev/null 2>&1 + -d "$PGC_DB" -v ON_ERROR_STOP=1 -Atq -c "SET statement_timeout='${PLAN_TIMEOUT}s';" -c "$1" -c "$2" >/dev/null 2>&1 out=$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ - -d "$PGC_DB" -Atq -c "SET statement_timeout='${PLAN_TIMEOUT}s';" -c "$1" -c '\timing on' -c "$2" 2>&1) - if grep -qiE 'timeout|canceling' <<<"$out"; then - printf '%s\tTIMEOUT\n' "${node:-unknown}"; return + -d "$PGC_DB" -v ON_ERROR_STOP=1 -Atq -c "SET statement_timeout='${PLAN_TIMEOUT}s';" -c "$1" -c '\timing on' -c "$2" 2>&1) + rc=$? + if [ "$rc" != 0 ]; then + if grep -q 'canceling statement due to statement timeout' <<<"$out"; then + printf '%s\tTIMEOUT\n' "${node:-unknown}" + else + out=$(grep -m1 -E 'ERROR|FATAL|error' <<<"$out" | tr '\t' ' ') + printf '%s\tERROR (%s)\n' "${node:-unknown}" "${out:-psql exit $rc}" + fi + return fi ms=$(grep -oE 'Time: [0-9.]+ ms' <<<"$out" | tail -1 | grep -oE '[0-9.]+') # Tab-delimited, because a node name contains spaces: "Custom Scan @@ -122,9 +137,15 @@ run_plan() { # run_plan -> " " } # The planner's choice against the best alternative it declined. +# +# THE CHECK NAMES CARRY THE LABEL AND NOTHING ELSE. They used to carry the bound, +# the timeout and the node that won the comparison, so the same check had one +# name when it passed ("... alternative (Index Scan)") and another when it failed +# ("... alternative (Custom Scan (PgColumnarScan))") -- a red that no green run +# had ever named. Those values are in the display line now, not the name. choice_vs_best() { # choice_vs_best