On 24.02.24 14:46, David E. Wheeler wrote:
> What’s the protocol for marking a patch ready for committer?
I guess after the review of the last assigned reviewer
v9 applies cleanly, all tests pass and documentation builds correctly.
Just a very small observation:
The fact that a completely invalid type returns NULL ..
SELECT to_regtypemod('foo');
to_regtypemod
---------------
(1 row)
.. but a "partially" valid one returns an error might be confusing
postgres=# SELECT to_regtypemod('timestamp(-4)');
ERROR: syntax error at or near "-"
LINE 1: SELECT to_regtypemod('timestamp(-4)');
^
CONTEXT: invalid type name "timestamp(-4)"
postgres=# SELECT to_regtypemod('text(-4)');
ERROR: type modifier is not allowed for type "text"
This behaviour is mentioned in the documentation, so I'd say it is ok.
+ <xref linkend="datatype-oid"/>). Failure to extract a valid
potential
+ type name results in an error; however, if the extracted name
is not
+ known to the system, this function will return
<literal>NULL</literal>.
I would personally prefer either NULL or an error in both cases, but I
can totally live with the current design.
OTOH, format_type returns "???" and it seems to be fine, so don't take
this comment too seriously :)
SELECT format_type(-1,-1);
format_type
-------------
???
(1 row)
Other than that, LGTM.
Thanks David!
Best,
Jim
--
Jim