Re: Patch: Add parse_type Function

Поиск
Список
Период
Сортировка
От jian he
Тема Re: Patch: Add parse_type Function
Дата
Msg-id CACJufxHS2ULcDvZJagS=RVo=Hqq_TC-jOG0euVdKvJ-DkcKtuw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Patch: Add parse_type Function  ("David E. Wheeler" <david@justatheory.com>)
Ответы Re: Patch: Add parse_type Function  (Erik Wienhold <ewie@ewie.name>)
Список pgsql-hackers
+ /*
+ * Parse type-name argument to obtain type OID and encoded typmod. We don't
+ * need to check for parseTypeString failure, but just let the error be
+ * raised. The 0 arg works both as the `Node *escontext` arg in Postgres 16
+ * and the `bool missing_ok` arg in 9.4-15.
+ */
+ (void) parseTypeString(type, &typid, &typmod, 0);
if you are wondering around other code deal with NULL, ErrorSaveContext.
NULL would be more correct?
`(void) parseTypeString(type, &typid, &typmod, NULL);`

also parseTypeString already explained the third argument, our
comments can be simplified as:
/*
* Parse type-name argument to obtain type OID and encoded typmod. We don't
* need to handle parseTypeString failure, just let the error be
* raised.
*/

cosmetic issue. Looking around other error handling code, the format
should be something like:
`
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
    ereport(ERROR,
                  (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                    errmsg("function returning record called in"
                                 "context that cannot accept type record")));
`

`PG_FUNCTION_INFO_V1(parse_type);`
is unnecessary?
based on the error information:  https://cirrus-ci.com/task/5899457502380032
not sure why it only fails on windows.

+#define PARSE_TYPE_STRING_COLS 2 /* Returns two columns. */
+#undef PARSE_TYPE_STRING_COLS
Are these necessary?
given that the comments already say return the OID and type modifier.


+        ( <parameter>typid</parameter> <type>oid</type>,
+          <parameter>typmod</parameter> <type>int4</type> )
here `int4` should be `integer`?

commit message:
`Also accounts for data typs that require the SQL grammar to be parsed:`
except the typo issue, this sentence is still hard for me to understand.

The `parse_type()` function uses the underlying `parseTypeString()` C
function to parse a string representing a data type into a type ID and
typmod suitabld for passing to `format_type()`.

suitabld should be suitable.


+       <para>
+        Parses a string representing an SQL type declaration as used in a
+        <command>CREATE TABLE</command> statement, optionally schema-qualified.
+        Returns a record with two fields, <parameter>typid</parameter> and
+        <parameter>typmod</parameter>, representing the OID and
modifier for the
+        type. These correspond to the parameters to pass to the
+        <link linkend="format_type"><function>format_type</function>
function.</link>
+       </para>

can be simplified:
+       <para>
+        Parses a string representing an SQL data type, optionally
schema-qualified.
+        Returns a record with two fields, <parameter>typid</parameter> and
+        <parameter>typmod</parameter>, representing the OID and
modifier for the
+        type. These correspond to the parameters to pass to the
+        <link linkend="format_type"><function>format_type</function>
function.</link>
+       </para>
(I don't have a strong opinion though)



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Noah Misch
Дата:
Сообщение: Re: Popcount optimization using AVX512
Следующее
От: Michael Paquier
Дата:
Сообщение: Re: Sequence Access Methods, round two