Обсуждение: Allow the identifier length to be increased via a configure option

I am sending the patch for the following TODO item:
Allow the identifier length to be increased via a configure option

During the configuration time, the user can set the identifier length.
./configure.txt --with-identlen=128

However, I am not clear about the requirement.
Can somebody review and comment on this?

Thanks
Dhanaraj

PS: The regression test (name.sql) needs some changes.

*** configure.in.orig   Wed Dec 27 14:56:10 2006
--- configure.in        Wed Dec 27 14:58:43 2006
***************
*** 179,184 ****
--- 179,196 ----
    AC_SUBST(default_port)

    #
+ # Identifier length ( --with-identlen), default 64 bytes
+ #
+ AC_MSG_CHECKING([for identifier length])
+ PGAC_ARG_REQ(with, identlen, [  --with-identlen=INT     identifier
length [[64 bytes]]],
+          [identlen=$withval],
+          [identlen=64])
+ AC_MSG_RESULT([$identlen])
+
+ sed 's/#define NAMEDATALEN .*/#define NAMEDATALEN '$identlen'/g'
./src/include/postgres_ext.h > ./src/include/postgres_ext.h.tmp
+ mv  ./src/include/postgres_ext.h.tmp ./src/include/postgres_ext.h
+
+ #
    # Option to disable shared libraries
    #
    PGAC_ARG_BOOL(enable, shared, yes,

Dhanaraj M wrote:
> I am sending the patch for the following TODO item:
> Allow the identifier length to be increased via a configure option
>
> During the configuration time, the user can set the identifier length.
> ./configure.txt --with-identlen=128
>
> However, I am not clear about the requirement.
> Can somebody review and comment on this?

You should use pg_config.h, not mangle postgres_ext.h like that.  Or
maybe generate postgres_ext.h from an hypotetical postgres_ext.h.in (but
I wouldn't do that, really).

--
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Dhanaraj M wrote:
>> I am sending the patch for the following TODO item:
>> Allow the identifier length to be increased via a configure option

> You should use pg_config.h, not mangle postgres_ext.h like that.  Or
> maybe generate postgres_ext.h from an hypotetical postgres_ext.h.in (but
> I wouldn't do that, really).

I'm wondering how this got into the TODO list.  It seems rather
pointless, and likely to create client compatibility problems (if not,
why is NAMEDATALEN exported at all?)

A bigger problem with the proposed patch is that it makes it look like
you can set the identifier length to any random number; the restrictions
mentioned in postgres_ext.h aren't enforced or even displayed to the
user of configure.

            regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
>
>> Dhanaraj M wrote:
>>
>>> I am sending the patch for the following TODO item:
>>> Allow the identifier length to be increased via a configure option
>>>
>
>
>> You should use pg_config.h, not mangle postgres_ext.h like that.  Or
>> maybe generate postgres_ext.h from an hypotetical postgres_ext.h.in (but
>> I wouldn't do that, really).
>>
>
> I'm wondering how this got into the TODO list.  It seems rather
> pointless, and likely to create client compatibility problems (if not,
> why is NAMEDATALEN exported at all?)
>
Will this TODO item be removed from the list?
Or I shall proceed with the suggestions given.

Thanks
Dhanaraj

Dhanaraj M wrote:
> Tom Lane wrote:
> > Alvaro Herrera <alvherre@commandprompt.com> writes:
> >
> >> Dhanaraj M wrote:
> >>
> >>> I am sending the patch for the following TODO item:
> >>> Allow the identifier length to be increased via a configure option
> >>>
> >
> >
> >> You should use pg_config.h, not mangle postgres_ext.h like that.  Or
> >> maybe generate postgres_ext.h from an hypotetical postgres_ext.h.in (but
> >> I wouldn't do that, really).
> >>
> >
> > I'm wondering how this got into the TODO list.  It seems rather
> > pointless, and likely to create client compatibility problems (if not,
> > why is NAMEDATALEN exported at all?)
> >
> Will this TODO item be removed from the list?
> Or I shall proceed with the suggestions given.

TODO item removed.

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Tom Lane wrote:
> I'm wondering how this got into the TODO list.  It seems rather
> pointless, and likely to create client compatibility problems (if
> not, why is NAMEDATALEN exported at all?)

I think because it used to be used in libpq's notification structure.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane wrote:
>> ... why is NAMEDATALEN exported at all?)

> I think because it used to be used in libpq's notification structure.

Yeah, you're probably right.  Maybe we should take it out of
postgres_ext.h and move it to pg_config_manual.h.  If no one complains
after a release cycle or so, we could reconsider making it configurable
more easily.

            regards, tom lane

Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > Tom Lane wrote:
> >> ... why is NAMEDATALEN exported at all?)
>
> > I think because it used to be used in libpq's notification structure.
>
> Yeah, you're probably right.  Maybe we should take it out of
> postgres_ext.h and move it to pg_config_manual.h.  If no one complains
> after a release cycle or so, we could reconsider making it configurable
> more easily.

Added to TODO:

    * Move NAMEDATALEN from postgres_ext.h to pg_config_manual.h and
      consider making it more configurable in future releases

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +