On Wed, Oct 21, 2020 at 01:59:50PM +0530, Dilip Kumar wrote:
On Sat, Oct 17, 2020 at 11:34 AM Dilip Kumar <dilipbal...@gmail.com> wrote:
On Tue, Oct 13, 2020 at 10:30 AM Dilip Kumar <dilipbal...@gmail.com> wrote:
>
> On Mon, Oct 12, 2020 at 7:32 PM Tomas Vondra
> <tomas.von...@2ndquadrant.com> wrote:
> >
> > On Mon, Oct 12, 2020 at 02:28:43PM +0530, Dilip Kumar wrote:
> > >
> > >> ...
> > >
> > >I have worked on this patch, so as discussed now I am maintaining the
> > >preserved compression methods using dependency. Still PRESERVE ALL
> > >syntax is not supported, I will work on that part.
> > >
> >
> > Cool, I'll take a look. What's your opinion on doing it this way? Do you
> > think it's cleaner / more elegant, or is it something contrary to what
> > the dependencies are meant to do?
>
> I think this looks much cleaner. Moreover, I feel that once we start
> supporting the custom compression methods then we anyway have to
> maintain the dependency so using that for finding the preserved
> compression method is good option.
I have also implemented the next set of patches.
0004 -> Provide a way to create custom compression methods
0005 -> Extention to implement lz4 as a custom compression method.
In the updated version I have worked on some of the listed items
A pending list of items:
1. Provide support for handling the compression option
- As discussed up thread I will store the compression option of the
latest compression method in a new field in pg_atrribute table
2. As of now I have kept zlib as the second built-in option and lz4 as
a custom compression extension. In Offlist discussion with Robert, he
suggested that we should keep lz4 as the built-in method and we can
move zlib as an extension because lz4 is faster than zlib so better to
keep that as the built-in method. So in the next version, I will
change that. Any different opinion on this?
Done
3. Improve the documentation, especially for create_compression_method.
4. By default support table compression method for the index.
Done
5. Support the PRESERVE ALL option so that we can preserve all
existing lists of compression methods without providing the whole
list.
1,3,5 points are still pending.
Thanks. I took a quick look at the patches and I think it seems fine. I
have one question, though - toast_compress_datum contains this code:
/* Call the actual compression function */
tmp = cmroutine->cmcompress((const struct varlena *) value);
if (!tmp)
return PointerGetDatum(NULL);
Shouldn't this really throw an error instead? I mean, if the compression
library returns NULL, isn't that an error?
regards
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
