Hi Chao, Appreciate you pulling it apart:
> 1
> Here when you call LookupTypeName(), you give the last parameter
> “missing_ok” a value of “false”, so that it would “ereport” inside
> LookupTypeName(), so your manual check of “if (typeTup == NULL)” will never
> be satisfied.
>
Yeah, I changed that to `true`, so it should handle correctly now - v3. But
not sure how you got "it would “ereport” inside LookupTypeName()". I don't
see where ereport would happen inside `LookupTypeNameExtended`. It seems
like it would hit here and return NULL:
if (!OidIsValid(typoid))
{
if (typmod_p)
*typmod_p = -1;
return NULL;
}
> 2
> Here you set proisstrict => ’t’. With strict mode, the function will not
> be executed if any of input arguments are NULL.
>
> So add this test seems meaningless, because the function is not executed
> at all.
> ```
> +SELECT pg_get_type_ddl(NULL);
> + pg_get_type_ddl
> +-----------------
> +
> +(1 row)
>
I added that test to make sure it provides no output.
3. As discussed in other get_xxx_ddl() patches, does this function needs a
> pretty flag? I think other patches have that.
>
I see three functions that a couple people posted that use it; however, for
pg_get_policy_ddl is the only one using it to format the code. I am not
sure there is consensus on how SQL should be formatted.
--
Best,
Phil Alger
v3-0001-Add-pg_get_type_ddl-function.patch
Description: Binary data
