From: | jian he <jian(dot)universality(at)gmail(dot)com> |
---|---|
To: | "David E(dot) Wheeler" <david(at)justatheory(dot)com> |
Cc: | Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>, Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
Subject: | Re: Patch: Add parse_type Function |
Date: | 2024-02-11 00:00:00 |
Message-ID: | CACJufxHS2ULcDvZJagS=RVo=Hqq_TC-jOG0euVdKvJ-DkcKtuw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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)
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2024-02-11 00:03:44 | Re: Sequence Access Methods, round two |
Previous Message | Noah Misch | 2024-02-10 23:52:38 | Re: Popcount optimization using AVX512 |