Re: [HACKERS] generic LONG VARLENA

> wieck@debis.com (Jan Wieck) writes:
> >     first  I  want  to  summarize  some details, to see if we all
> >     agree so far in the discussion.
> 
> I snipped everything I agreed with ;-)
> 
> >     - The implementation should be generic for all variable  size
> >       types, but can be enabled/disabled per type.
>                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> Per-type control doesn't strike me as interesting or useful.  If there
> needs to be a control at all, which I doubt, per-table would be the
> way to go.  But how many users will really say, "Oh yes, I *want* the
> thing to fail if my tuple's too big!"?  I say: make it automatically
> apply whenever needed, don't force users to think about it.

Agreed.  Who wouldn't want it.

> 
> >     - The type specific functions call a central support function
> >       to  get the usual VARLENA format, which is taken from a LRU
> >       cache or fetched from  the  extension  relation.  They  are
> >       responsible  for freeing the memory after they're done with
> >       the value.
> 
> If we are going to do this, we ought also think about solving the
> generic memory-leakage problem at the same time.  No point in having
> to revisit all the same code later to deal with that issue.

I have a good fix for this.  My patch suggested the varlena routine
pfree the pointer returned from expand_long().  No need for that.  With
an LRU cache, we can have the cache itself free the old values.  This
would be a nice optimization.  Just add the lines below:

+ 
+     if (VARISLONG(vlena)) /* checks long bit */
+         vlena = expand_long(vlena); /* returns palloc long */
+ 

There aren't any cases where the varlena access routines access more
than two varlena values at the same time.  If the expansion cache is at
least two values, you can just expand it and return memory.  When that
cache entry is expired, the memory is freed.  Wow, this makes the
varlena changes very compact.  All the action is in expand_long().

Basically, don't have the access routines free the memory, have the old
cache entries be pfreed.

> 
> >     I  don't  think  it  is  a  good idea to create the expansion
> >     relation all the time. Some keyword in CREATE  TABLE,  and/or
> >     another ALTER TABLE should do it instead, so the DB admin can
> >     activate the LONG feature on a per table base as  needed.
> 
> I don't believe it.  See above: people will complain that it's a bug
> that the system doesn't handle their long data values.  Saying "oh, you
> have to turn it on" will not appease them.  My objection is really the
> same as for the specialized LONG datatype: I do *not* want people to
> have to put nonstandard junk into their database schema declarations
> in order to activate this feature.  I think it should Just Work and
> stay out of users' faces.

> Creating the expansion relation isn't that big a deal, but if you
> don't want to do it always, why not do it on first use?
> 

Yes, why not just create it the first time it is needed.  Seems pretty
small performance-wise.

> >     Also  I  would  like to say that system relations cannot have
> >     expansion relations.  At  least  not  until  we  have  enough
> >     experience with this stuff.
> 
> I'd really, really, really like to have this work for rules, though.
> Why shouldn't we allow it for system relations?  Most of the critical
> ones have fixed-width tuples anyway, so it won't matter for them.

Oh, that's a good point.  Seems that is a big reason for expansion of
types.

> 
> BTW, it strikes me we should drop the "lztext" special datatype, and
> instead have compression automatically applied to any varlena that
> we are contemplating putting out-of-line.  (If we're really lucky,
> that saves us having to put the value out-of-line!)

Ooh, very smart.  You would need another bit to say whether the varlena
is compressed or now.  If you take it from 4-byte header, we are down to
a 1 GB length limit.  You could do all the compression/decompression in
the two expansion functions, though compressing and then not using the
long_ table would be a little tricky to code, but do-able.  You would
compress, then if still too large, move to long table.



> 
> >     Is  that  now  what we initially want to give a try? If so, I
> >     would like to start soon to get the generic part ready  ASAP.
> >     Others  could  then  join  in  and  contribute by adding LONG
> >     support for all the VARLENA data types we have.
> 
> Yes, if we don't do it inside fastgetattr then there's a lot of code
> that will have to change.

See above.  It looks like only a few lines per function now.  If we do
it in fastgetattr, is there less code to change?  How do we pfree()?

> 
> >     Would really be a big leap if we can get this finished for  a
> >     reasonable number of VARLENA types by February 1.
> 
> The more I think about this the more I think that it's a bad, bad idea
> to try to have it ready by Feb 1.  There's not really enough time to
> get it right and test it.  I don't want to be putting out an unstable
> release, and that's what I'm afraid we'll have if we try to rush in
> such a major change as this.  Particularly when we have nontrivial
> amounts of unfinished business elsewhere that we shouldn't neglect.
> (Jan, do you really think you can make this happen *and* bring foreign
> keys to a finished status before February?  If you are going to leave
> stuff undone in foreign keys, I think you are making the wrong choice.)
> 
> Furthermore, we can save ourselves some time if we tackle this change
> in combination with the fmgr revision and the memory-leak-elimination
> issue.  We will be touching all the same per-data-type code for each
> of these issues, so why not touch it once instead of several times?
> 
> In short, I like this design but I think we should plan it for 7.1.

Not sure on this one.  Jan will have to comment.

I am excited about the long data type.  This is _the_ way to do long
data types.  Have any of the commercial databases figured out this way
to do it.  I can't imagine a better system.

--  Bruce Momjian                        |  http://www.op.net/~candle maillist@candle.pha.pa.us            |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026