Hi,
Tom Lane <[email protected]> writes:
> They are adding a large amount of new code that will have to be
> kept in sync with behavior elsewhere, and there is basically zero
> forcing function to ensure that that happens.
Agree. For the sake of completeness I did a thorough pass over the
rest of v7 and found a few more issues. Documenting them here so
they're on the record regardless of where the broader discussion
about the approach lands.
1) get_formatted_string silently drops large formatted strings
+ va_start(args, fmt);
+ appendStringInfoVA(buf, fmt, args);
+ va_end(args);
appendStringInfoVA returns non-zero when the buffer is too small,
requiring enlargeStringInfo + retry (see appendStringInfo in
stringinfo.c). The return value is ignored here, so large
formatted text is silently lost.
Reproduction -- a domain with a ~2647-char CHECK expression:
DO $$
DECLARE long_check text;
BEGIN
long_check := 'CHECK (';
FOR i IN 1..50 LOOP
IF i > 1 THEN long_check := long_check || ' OR '; END IF;
long_check := long_check || format(
'VALUE ~ ''^pattern_%s_[a-zA-Z0-9]{10,20}$''', i);
END LOOP;
long_check := long_check || ')';
EXECUTE format(
'CREATE DOMAIN huge_domain AS text CONSTRAINT big_check %s',
long_check);
END $$;
select pg_get_domain_ddl('huge_domain');
CREATE DOMAIN public.huge_domain AS pg_catalog.text CONSTRAINT big_check ;
(1 row)
The entire CHECK clause (~2647 chars) is silently dropped.
This function was adopted from the pg_get_policy_ddl patch [1].
I checked v8 there and confirmed the same bug exists.
2) Function is VOLATILE PARALLEL UNSAFE
pg_proc.dat is missing provolatile => 's', and system_functions.sql
does not specify STABLE PARALLEL SAFE. Every other pg_get_*def
function is STABLE PARALLEL SAFE:
select proname, provolatile, proparallel from pg_proc
where proname in ('pg_get_domain_ddl','pg_get_constraintdef',
'pg_get_functiondef','pg_get_triggerdef');
pg_get_constraintdef | s | s
pg_get_domain_ddl | v | u <--
pg_get_functiondef | s | s
pg_get_triggerdef | s | s
Same issue in pg_get_policy_ddl v8 [1].
3) Internal type names exposed (related to typmod bug)
generate_qualified_type_name also uses the raw pg_type.typname,
so beyond losing modifiers:
int[] -> pg_catalog._int4 (should be integer[])
char(5) -> pg_catalog.bpchar (should be character(5))
timestamp(6) -> pg_catalog."timestamp" (should be timestamp(6) without time
zone)
The format_type_extended fix from my earlier message resolves
this too. Several test expectations in object_ddl.out will need
updating once fixed.
Regards,
Haritabh
[1]
https://www.postgresql.org/message-id/flat/CANxoLDdJsRJqnjMXV3yjsk07Z5iRWxG-c2hZJC7bAKqf8ZXj_A%40mail.gmail.com
