On Thu, Apr 07, 2022 at 08:39:58PM +0900, Masahiko Sawada wrote:
> On Thu, Apr 7, 2022 at 7:28 PM Amit Kapila <amit.kapil...@gmail.com> wrote:
> > On Thu, Apr 7, 2022 at 8:25 AM Amit Kapila <amit.kapil...@gmail.com> wrote:
> > > I'll take care of this today. I think we can mark the new function
> > > get_column_offset() being introduced by this patch as parallel safe.
> >
> > Pushed.
>
> Thanks!
I took a closer look at the test case. The "get_column_offset(coltypes) % 8"
part would have caught the problem only when run on an ALIGNOF_DOUBLE==4
platform. Instead of testing the start of the typalign='d' column, let's test
the first offset beyond the previous column. The difference between those two
values depends on ALIGNOF_DOUBLE. While there, ignore typbyval; it doesn't
affect disk tuple layout, so this test shouldn't care. I plan to push the
attached patch.
Author: Noah Misch <n...@leadboat.com>
Commit: Noah Misch <n...@leadboat.com>
Test ALIGNOF_DOUBLE==4 compatibility under ALIGNOF_DOUBLE==8.
Today's test case detected alignment problems only when executing on
AIX. This change lets popular platforms detect the same problems.
Reviewed by FIXME.
Discussion: https://postgr.es/m/FIXME
diff --git a/src/test/regress/expected/sanity_check.out
b/src/test/regress/expected/sanity_check.out
index a2faefb..c5c675b 100644
--- a/src/test/regress/expected/sanity_check.out
+++ b/src/test/regress/expected/sanity_check.out
@@ -39,18 +39,18 @@ WITH check_columns AS (
SELECT t.oid
FROM pg_type t JOIN pg_attribute pa ON t.oid = pa.atttypid
WHERE pa.attrelid = a.attrelid AND
- pa.attnum > 0 AND pa.attnum <= a.attnum
+ pa.attnum > 0 AND pa.attnum < a.attnum
ORDER BY pa.attnum) AS coltypes
FROM pg_attribute a JOIN pg_class c ON c.oid = attrelid
JOIN pg_namespace n ON c.relnamespace = n.oid
WHERE attalign = 'd' AND relkind = 'r' AND
attnotnull AND attlen <> -1 AND n.nspname = 'pg_catalog'
)
-SELECT relname, attname, coltypes, get_column_offset(coltypes)
+SELECT relname, attname, coltypes, get_columns_length(coltypes)
FROM check_columns
- WHERE get_column_offset(coltypes) % 8 != 0 OR
+ WHERE get_columns_length(coltypes) % 8 != 0 OR
'name'::regtype::oid = ANY(coltypes);
- relname | attname | coltypes | get_column_offset
----------+---------+----------+-------------------
+ relname | attname | coltypes | get_columns_length
+---------+---------+----------+--------------------
(0 rows)
diff --git a/src/test/regress/expected/test_setup.out
b/src/test/regress/expected/test_setup.out
index 8b8ba7d..391b36d 100644
--- a/src/test/regress/expected/test_setup.out
+++ b/src/test/regress/expected/test_setup.out
@@ -206,7 +206,7 @@ CREATE FUNCTION ttdummy ()
RETURNS trigger
AS :'regresslib'
LANGUAGE C;
-CREATE FUNCTION get_column_offset (oid[])
+CREATE FUNCTION get_columns_length(oid[])
RETURNS int
AS :'regresslib'
LANGUAGE C STRICT STABLE PARALLEL SAFE;
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index 8b0c2d9..ade4b51 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -1219,12 +1219,12 @@ binary_coercible(PG_FUNCTION_ARGS)
}
/*
- * Return the column offset of the last data in the given array of
- * data types. The input data types must be fixed-length data types.
+ * Return the length of the portion of a tuple consisting of the given array
+ * of data types. The input data types must be fixed-length data types.
*/
-PG_FUNCTION_INFO_V1(get_column_offset);
+PG_FUNCTION_INFO_V1(get_columns_length);
Datum
-get_column_offset(PG_FUNCTION_ARGS)
+get_columns_length(PG_FUNCTION_ARGS)
{
ArrayType *ta = PG_GETARG_ARRAYTYPE_P(0);
Oid *type_oids;
@@ -1249,14 +1249,10 @@ get_column_offset(PG_FUNCTION_ARGS)
get_typlenbyvalalign(typeoid, &typlen, &typbyval, &typalign);
/* the data type must be fixed-length */
- if (!(typbyval || (typlen > 0)))
+ if (typlen < 0)
elog(ERROR, "type %u is not fixed-length data type",
typeoid);
- column_offset = att_align_nominal(column_offset, typalign);
-
- /* not include the last type size */
- if (i != (ntypes - 1))
- column_offset += typlen;
+ column_offset = att_align_nominal(column_offset + typlen,
typalign);
}
PG_RETURN_INT32(column_offset);
diff --git a/src/test/regress/sql/sanity_check.sql
b/src/test/regress/sql/sanity_check.sql
index c70ff78..7f338d1 100644
--- a/src/test/regress/sql/sanity_check.sql
+++ b/src/test/regress/sql/sanity_check.sql
@@ -34,14 +34,14 @@ WITH check_columns AS (
SELECT t.oid
FROM pg_type t JOIN pg_attribute pa ON t.oid = pa.atttypid
WHERE pa.attrelid = a.attrelid AND
- pa.attnum > 0 AND pa.attnum <= a.attnum
+ pa.attnum > 0 AND pa.attnum < a.attnum
ORDER BY pa.attnum) AS coltypes
FROM pg_attribute a JOIN pg_class c ON c.oid = attrelid
JOIN pg_namespace n ON c.relnamespace = n.oid
WHERE attalign = 'd' AND relkind = 'r' AND
attnotnull AND attlen <> -1 AND n.nspname = 'pg_catalog'
)
-SELECT relname, attname, coltypes, get_column_offset(coltypes)
+SELECT relname, attname, coltypes, get_columns_length(coltypes)
FROM check_columns
- WHERE get_column_offset(coltypes) % 8 != 0 OR
+ WHERE get_columns_length(coltypes) % 8 != 0 OR
'name'::regtype::oid = ANY(coltypes);
diff --git a/src/test/regress/sql/test_setup.sql
b/src/test/regress/sql/test_setup.sql
index fbceb8c..02c0c84 100644
--- a/src/test/regress/sql/test_setup.sql
+++ b/src/test/regress/sql/test_setup.sql
@@ -253,7 +253,7 @@ CREATE FUNCTION ttdummy ()
AS :'regresslib'
LANGUAGE C;
-CREATE FUNCTION get_column_offset (oid[])
+CREATE FUNCTION get_columns_length(oid[])
RETURNS int
AS :'regresslib'
LANGUAGE C STRICT STABLE PARALLEL SAFE;
