On 2025-Jun-25, Fujii Masao wrote:
> > however, in determineNotNullFlags we have:
> >
> > char *default_name;
> > /* XXX should match ChooseConstraintName better */
> > default_name = psprintf("%s_%s_not_null",
> > tbinfo->dobj.name,
> > tbinfo->attnames[j]);
> > if (strcmp(default_name,
> > PQgetvalue(res, r, i_notnull_name)) == 0)
> > tbinfo->notnull_constrs[j] = "";
> Do we really need this part? With the patch, due to this part,
> pg_dump might emit output like:
>
> CREATE TABLE t (i int NOT NULL);
> COMMENT ON CONSTRAINT t_i_not_null ON public.t IS '.......';
>
> This seems to rely on the assumption that the naming convention for
> NOT NULL constraints (e.g., t_i_not_null) will remain stable across versions.
Yeah, I think in this case we need to extract the constraint name so
that we have it available to print the COMMENT command, rather than
making any assumptions about it. In fact I suspect this would fail if
the table or column names are very long. For the other pg_dump uses of
this logic it doesn't matter AFAIR, but here I think we must be
stricter.
> Wouldn't it be safer to always print the constraint name explicitly,
> even if it's the default name?
Yes, but we wanted to avoid printing those names in "normal cases," so
we go some lengths to avoid them if not necessary. However, I suspect
it's easy to print the names only if a comment exists -- which is also
an unusual case.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"XML!" Exclaimed C++. "What are you doing here? You're not a programming
language."
"Tell that to the people who use me," said XML.
https://burningbird.net/the-parable-of-the-languages/
