Re: current version: Patch - Have psql show current values

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Dhanaraj M <Dhanaraj(dot)M(at)Sun(dot)COM>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-patches(at)postgresql(dot)org
Subject: Re: current version: Patch - Have psql show current values
Date: 2006-05-06 02:52:03
Message-ID: 200605060252.k462q3r20899@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches


As Tom asked, why not use seqname.last_value? Looking at your output:

+ if (showSeq && !showTables)
+ appendPQExpBuffer(&buf,
+ ",\n curval(c.oid) as \"%s\""
+ ",\n CASE curvalcheck(c.oid) WHEN '1' THEN '%s' WHEN '0' THEN '%s' END as \"%s\"",
+ _("value"),_(" ***"),_(""),_("Start from"));

What do you want to show that seqname.last_value doesn't give you?
Curval? I don't see that as useful for a psql display. Now that I look
at the TODO item:

o Have psql show current values for a sequence

It is confusing. It means "the current values" for the sequence, not
"curval" for the sequence. I don't even understand what your function
is returning. Just stick to last_value, though I think
seqname.is_called might be what you were looking for.

What fields do we want to show? Maybe the TODO item is not needed. Is
this all we want to show?

test=> \x
Expanded display is on.

test=> select * from xx;
-[ RECORD 1 ]-+--------------------
sequence_name | xx
last_value | 1
increment_by | 1
max_value | 9223372036854775807
min_value | 1
cache_value | 1
log_cnt | 32
is_cycled | f
is_called | t

---------------------------------------------------------------------------

Dhanaraj M wrote:
> Tom Lane wrote:
>
> >Dhanaraj M <Dhanaraj(dot)M(at)Sun(dot)COM> writes:
> >
> >
> >>sorry for sending the old version in the previous mail . Here I attach
> >>the recent version of the patch file.
> >>
> >>
> >----------------------------------------------------------------------
> >
> >
> Surely this problem does not require adding any server-side code.
>
> >Something like "select last_value from <seq>" would be more appropriate;
> >and it'd have some hope of working with back-version servers.
> >
> >Also, please use something more helpful than "***" as the column
> >header. Your urge to add a footnote to explain it shows that you
> >didn't try hard enough to devise a good header to begin with.
> >
> >[ btw, both fmgroids.h and fmgrtab.c are generated files. Patching
> >them is unnecessary and inappropriate. ]
> >
> ----------------------------------------------------------
>
> The existing functions like lastval, currval dont provide the current
> sequence value always.
> They work only if the sequence is already cached (nextval is called
> atleast once for that sequence).
> Changing the internals of lastval/currval will give the solution.
> However, I feel that the functionality change
> may affect the customers who use the current version.
>
> Hence, I am sure that it requires the server side change. There are two
> options here
> 1. Modifying the exisitng functions (or) 2. adding new functions
>
>
> Thanks for your review
> Dhanaraj
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings
>

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +
Dhanaraj M wrote:
> Tom Lane wrote:
>
> >Dhanaraj M <Dhanaraj(dot)M(at)Sun(dot)COM> writes:
> >
> >
> >>sorry for sending the old version in the previous mail . Here I attach
> >>the recent version of the patch file.
> >>
> >>
> >----------------------------------------------------------------------
> >
> >
> Surely this problem does not require adding any server-side code.
>
> >Something like "select last_value from <seq>" would be more appropriate;
> >and it'd have some hope of working with back-version servers.
> >
> >Also, please use something more helpful than "***" as the column
> >header. Your urge to add a footnote to explain it shows that you
> >didn't try hard enough to devise a good header to begin with.
> >
> >[ btw, both fmgroids.h and fmgrtab.c are generated files. Patching
> >them is unnecessary and inappropriate. ]
> >
> ----------------------------------------------------------
>
> The existing functions like lastval, currval dont provide the current
> sequence value always.
> They work only if the sequence is already cached (nextval is called
> atleast once for that sequence).
> Changing the internals of lastval/currval will give the solution.
> However, I feel that the functionality change
> may affect the customers who use the current version.
>
> Hence, I am sure that it requires the server side change. There are two
> options here
> 1. Modifying the exisitng functions (or) 2. adding new functions
>
>
> Thanks for your review
> Dhanaraj
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings
>

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Euler Taveira de Oliveira 2006-05-06 03:50:25 Re: current version: Patch - Have psql show current values
Previous Message Chuck McDevitt 2006-05-05 23:21:16 Re: [pgsql-hackers-win32] Build with Visual Studio &

Browse pgsql-patches by date

  From Date Subject
Next Message Euler Taveira de Oliveira 2006-05-06 03:50:25 Re: current version: Patch - Have psql show current values
Previous Message Bruce Momjian 2006-05-06 02:33:36 Re: cast bytea to/from bit strings