On 2021-Mar-19, Robert Haas wrote:
> Here's one from me that tries to make the handling of the LZ4 stuff
> more like what we already do for zlib, but I'm not sure if it's
> correct, or if it's what everyone wants.
This one seems to behave as expected (Debian 10, with and without
liblz4-dev).
--
On Fri, Mar 19, 2021 at 10:19:49PM +0100, Tomas Vondra wrote:
> On 3/19/21 9:40 PM, Andres Freund wrote:
> > On 2021-03-19 17:35:58 -0300, Alvaro Herrera wrote:
> >> I find this behavior confusing; I'd rather have configure error out if
> >> it can't find the package support I requested, than
On 2021-Mar-19, Robert Haas wrote:
> On Fri, Mar 19, 2021 at 10:11 AM Dilip Kumar wrote:
> > Also added a test case for vacuum full to recompress the data.
>
> I committed the core patch (0003) with a bit more editing. Let's see
> what the buildfarm thinks.
I updated the coverage script to
On 2021-Mar-19, Robert Haas wrote:
> > Regarding your point, that does look like clutter. We don't annotate
> > the dump with a storage clause unless it's non-default, so probably we
> > should do the same thing here. I think I gave Dilip bad advice here...
>
> Here's a patch for that. It's a
On Fri, Mar 19, 2021 at 5:29 PM Andres Freund wrote:
> On 2021-03-19 22:19:49 +0100, Tomas Vondra wrote:
> > Yeah. And why does it even require pkg-config, unlike any other library
> > that I'm aware of?
>
> IMO it's fine to require pkg-config to simplify the configure
> code. Especially for new
On Fri, Mar 19, 2021 at 4:38 PM Robert Haas wrote:
> Yes, and prion's got this concerning diff:
>
> Column | Type | Collation | Nullable | Default | Storage |
> Compression | Stats target | Description
>
>
On 2021-03-19 22:19:49 +0100, Tomas Vondra wrote:
> Yeah. And why does it even require pkg-config, unlike any other library
> that I'm aware of?
IMO it's fine to require pkg-config to simplify the configure
code. Especially for new optional features. Adding multiple alternative
ways to discover
On 3/19/21 9:40 PM, Andres Freund wrote:
> On 2021-03-19 17:35:58 -0300, Alvaro Herrera wrote:
>> I find this behavior confusing; I'd rather have configure error out if
>> it can't find the package support I requested, than continuing with a
>> set of configure options different from what I gave.
On 2021-03-19 17:35:58 -0300, Alvaro Herrera wrote:
> I find this behavior confusing; I'd rather have configure error out if
> it can't find the package support I requested, than continuing with a
> set of configure options different from what I gave.
+1
On Fri, Mar 19, 2021 at 4:20 PM Tom Lane wrote:
> Nope ... crake's displeased with your assumption that it's OK to
> clutter dumps with COMPRESSION clauses. As am I: that is going to
> be utterly fatal for cross-version transportation of dumps.
Yes, and prion's got this concerning diff:
Hmm, if I use configure --with-lz4, I get this:
checking whether to build with LZ4 support... yes
checking for liblz4... no
configure: error: Package requirements (liblz4) were not met:
No package 'liblz4' found
Consider adjusting the PKG_CONFIG_PATH
I wrote:
> Since no animals will be using --with-lz4, I'd expect vast silence.
Nope ... crake's displeased with your assumption that it's OK to
clutter dumps with COMPRESSION clauses. As am I: that is going to
be utterly fatal for cross-version transportation of dumps.
Robert Haas writes:
> I committed the core patch (0003) with a bit more editing. Let's see
> what the buildfarm thinks.
Since no animals will be using --with-lz4, I'd expect vast silence.
regards, tom lane
On Fri, Mar 19, 2021 at 10:11 AM Dilip Kumar wrote:
> Also added a test case for vacuum full to recompress the data.
I committed the core patch (0003) with a bit more editing. Let's see
what the buildfarm thinks.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Fri, Mar 19, 2021 at 1:44 PM Justin Pryzby wrote:
> Working with one of Andrey's patches on another thread, he reported offlist
> getting this message, resolved by this patch. Do you see this warning during
> ./configure ? The latest CI is of a single patch without the LZ4 stuff, so I
>
On Fri, Mar 19, 2021 at 01:24:39PM -0400, Robert Haas wrote:
> On Fri, Mar 19, 2021 at 12:35 PM Justin Pryzby wrote:
> > I sent offlist a couple of times but notice that the latest patch is missing
> > this bit around AC_CHECK_HEADERS, which apparently can sometimes cause
> > warnings on mac.
> >
On Fri, Mar 19, 2021 at 12:35 PM Justin Pryzby wrote:
> I sent offlist a couple of times but notice that the latest patch is missing
> this bit around AC_CHECK_HEADERS, which apparently can sometimes cause
> warnings on mac.
>
> ac_save_CPPFLAGS=$CPPFLAGS
> CPPFLAGS="$LZ4_CFLAGS $CPPFLAGS"
>
I sent offlist a couple of times but notice that the latest patch is missing
this bit around AC_CHECK_HEADERS, which apparently can sometimes cause
warnings on mac.
ac_save_CPPFLAGS=$CPPFLAGS
CPPFLAGS="$LZ4_CFLAGS $CPPFLAGS"
AC_CHECK_HEADERS(lz4/lz4.h, [],
[AC_CHECK_HEADERS(lz4.h, [],
On Thu, Mar 18, 2021 at 10:22 AM Dilip Kumar wrote:
> I just realized that in the last patch (0003) I forgot to remove 2
> unused functions, CompressionMethodToId and CompressionIdToMethod.
> Removed in the latest patch.
I spent a little time polishing 0001 and here's what I came up with. I
On Wed, Mar 17, 2021 at 02:50:34PM -0700, Andres Freund wrote:
> On 2021-03-17 16:01:58 -0400, Robert Haas wrote:
> > > - why is HIDE_TOAST_COMPRESSION useful? Doesn't quite seem to be
> > > comparable to HIDE_TABLEAM?
> >
> > Andres, what do you mean by this exactly? It's exactly the same
Hi,
On 2021-03-17 16:01:58 -0400, Robert Haas wrote:
> > - why is HIDE_TOAST_COMPRESSION useful? Doesn't quite seem to be
> > comparable to HIDE_TABLEAM?
>
> Andres, what do you mean by this exactly? It's exactly the same issue:
> without this, if you change the default compression method,
).On Mon, Mar 15, 2021 at 6:58 PM Andres Freund wrote:
> - Adding all these indirect function calls via toast_compression[] just
> for all of two builtin methods isn't fun either.
Yeah, it feels like this has too many layers of indirection now. Like,
toast_decompress_datum() first gets
On Wed, Mar 17, 2021 at 2:17 PM Andres Freund wrote:
> OTOH heap_form_flattened_tuple() has the advantage that we can optimize
> it further (e.g. to do the conversion to flattened values in fill_val())
> without changing the outside API.
Well, in my view, that does change the outside API,
Hi,
On 2021-03-17 13:31:14 -0400, Robert Haas wrote:
> On Wed, Mar 17, 2021 at 7:41 AM Dilip Kumar wrote:
> > 0002:
> > - Wrapper over heap_form_tuple and used in ExecEvalRow() and
> > ExecEvalFieldStoreForm()
>
> Instead of having heap_form_flattened_tuple(), how about
>
On Wed, Mar 17, 2021 at 7:41 AM Dilip Kumar wrote:
> 0002:
> - Wrapper over heap_form_tuple and used in ExecEvalRow() and
> ExecEvalFieldStoreForm()
Instead of having heap_form_flattened_tuple(), how about
heap_flatten_values(tupleDesc, values, isnull) that is documented to
modify the values
On Tue, Mar 16, 2021 at 2:54 PM Andres Freund wrote:
> Oh, I guess it would make sense to do it that way. However, I was just
> thinking of doing the iteration over the tuples that ExecEvalRow() etc
> do inside heap_form_flattened_tuple() (or whatever). That'd not be any
> worse than what the
On Tue, Mar 16, 2021 at 11:21 AM Dilip Kumar wrote:
> If that is only the argument then it's possible today as well. I mean
> you can INSERT INTO .. SELECT FROM where source attribute as
> compressed data but the target attribute as external storage then also
> we will move the compressed data
Hi,
On 2021-03-16 10:27:12 -0400, Robert Haas wrote:
> > I'm don't like that after 0002 ExecEvalRow(), ExecEvalFieldStoreForm()
> > contain a nearly identical copy of the same code. And
> > make_tuple_from_row() also is similar. It seem that there should be a
> > heap_form_tuple() version doing
On Tue, Mar 16, 2021 at 7:57 PM Robert Haas wrote:
>
> That behavior feels unacceptable to me from a user expectations point
> of view. I think there's an argument that if I update a tuple that
> contains a compressed datum, and I don't update that particular
> column, I think it would be OK to
On Mon, Mar 15, 2021 at 6:58 PM Andres Freund wrote:
> I don't particularly like PG_RETURN_HEAPTUPLEHEADER_RAW(). What is "raw"
> about it? It also seems to me like there needs to at least be a
> sentence or two explaining when to use which of the functions.
It seemed like the natural name to
On Tue, Mar 16, 2021 at 4:07 PM Dilip Kumar wrote:
>
> INSERT TIME
> Head: 17418.299 ms Patch: 20956.231 ms
>
> CTAS TIME:
> Head: 12837.872 ms Patch: 16775.739 ms
On quick analysis with perf it appeared that the performance is
degrading because of deforming
- 16.19% 3.54% postgres
On Tue, Mar 16, 2021 at 12:59 AM Robert Haas wrote:
>
> On Mon, Mar 15, 2021 at 8:14 AM Dilip Kumar wrote:
> > In the attached patches I have changed this, ...
>
> OK, so just looking over this patch series, here's what I think:
>
> Regarding 0003:
>
> The biggest thing that jumps out at me
On Tue, Mar 16, 2021 at 11:18 AM Justin Pryzby wrote:
>
> I'm a minor contributor now to a couple bits of this patch set, but I can
> answer a couple of these points.
>
> On Mon, Mar 15, 2021 at 03:58:35PM -0700, Andres Freund wrote:
> > Comments about 0003:
> > - why is HIDE_TOAST_COMPRESSION
I'm a minor contributor now to a couple bits of this patch set, but I can
answer a couple of these points.
On Mon, Mar 15, 2021 at 03:58:35PM -0700, Andres Freund wrote:
> Comments about 0003:
> - why is HIDE_TOAST_COMPRESSION useful? Doesn't quite seem to be
> comparable to HIDE_TABLEAM?
That
On Tue, Mar 16, 2021 at 4:28 AM Andres Freund wrote:
Replying to some of the comments..
> - Is nodeModifyTable.c really the right place for the logic around
> CompareCompressionMethodAndDecompress()? And is doing it in every
> place that does "user initiated" inserts really the right way?
Hi,
On 2021-03-15 15:29:05 -0400, Robert Haas wrote:
> On Mon, Mar 15, 2021 at 8:14 AM Dilip Kumar wrote:
> > In the attached patches I have changed this, ...
>
> OK, so just looking over this patch series, here's what I think:
>
> - 0001 and 0002 are now somewhat independent of the rest of this
On Mon, Mar 15, 2021 at 8:14 AM Dilip Kumar wrote:
> In the attached patches I have changed this, ...
OK, so just looking over this patch series, here's what I think:
- 0001 and 0002 are now somewhat independent of the rest of this work,
and could be dropped, but I think they're a good idea, so
I think these names need to be more specific.
+typedef enum CompressionId
+{
+PGLZ_COMPRESSION_ID = 0,
+LZ4_COMPRESSION_ID = 1
+
CompressionId, PGLZ_COMPRESSION_ID, LZ4_COMPRESSION_ID are also being used by
Andrey's WAL compression patch. I suggested he use a prefix, but your
On Fri, Mar 12, 2021 at 2:12 PM Dilip Kumar wrote:
>
> On Fri, Mar 12, 2021 at 10:45 AM Justin Pryzby wrote:
> >
> > On Thu, Mar 11, 2021 at 12:25:26PM -0600, Justin Pryzby wrote:
> > > On Wed, Mar 10, 2021 at 08:28:58PM -0600, Justin Pryzby wrote:
> > > > This includes a patch to use pkgconfig,
On Fri, Mar 12, 2021 at 10:45 AM Justin Pryzby wrote:
>
> On Thu, Mar 11, 2021 at 12:25:26PM -0600, Justin Pryzby wrote:
> > On Wed, Mar 10, 2021 at 08:28:58PM -0600, Justin Pryzby wrote:
> > > This includes a patch to use pkgconfig, in an attempt to build on mac,
> > > which
> > > currently
On Fri, Mar 12, 2021 at 11:55 AM Justin Pryzby wrote:
>
> It worked everywhere (but everytime someone mail this thread, it queues a
> rebuild).
> http://cfbot.cputube.org/dilip-kumar.html
Okay
> HAVE_LIBLZ4 was being set by AC_CHECK_LIB(), which is no longer used in favour
> of pkgconfig.
>
On Fri, Mar 12, 2021 at 11:24:57AM +0530, Dilip Kumar wrote:
> On Fri, Mar 12, 2021 at 10:45 AM Justin Pryzby wrote:
> >
> > On Thu, Mar 11, 2021 at 12:25:26PM -0600, Justin Pryzby wrote:
> > > On Wed, Mar 10, 2021 at 08:28:58PM -0600, Justin Pryzby wrote:
> > > > This includes a patch to use
On Fri, Mar 12, 2021 at 10:45 AM Justin Pryzby wrote:
>
> On Thu, Mar 11, 2021 at 12:25:26PM -0600, Justin Pryzby wrote:
> > On Wed, Mar 10, 2021 at 08:28:58PM -0600, Justin Pryzby wrote:
> > > This includes a patch to use pkgconfig, in an attempt to build on mac,
> > > which
> > > currently
On Fri, Mar 12, 2021 at 2:54 AM Justin Pryzby wrote:
>
> On Thu, Mar 11, 2021 at 10:07:30AM +0530, Dilip Kumar wrote:
> > On Thu, Mar 11, 2021 at 8:50 AM Justin Pryzby wrote:
> > >
> > > Looking at v23-0002-alter-table-set-compression, ATRewriteTable() was
> > > calling
> > >
On Fri, Mar 12, 2021 at 9:03 AM Justin Pryzby wrote:
>
> On Fri, Mar 12, 2021 at 08:38:41AM +0530, Dilip Kumar wrote:
> > On Thu, Mar 11, 2021 at 11:55 PM Justin Pryzby wrote:
> > > On Wed, Mar 10, 2021 at 08:28:58PM -0600, Justin Pryzby wrote:
> > > > This includes a patch to use pkgconfig, in
On Fri, Mar 12, 2021 at 8:54 AM Justin Pryzby wrote:
>
> +#elif LZ4_VERSION_NUMBER < 10803
> + return lz4_cmdecompress(value);
> +#else
>
> It occurred to me that this should actually compare the runtime version with
> LZ4_versionNumber(). That way, a library upgrade can enable the slice
>
On Fri, Mar 12, 2021 at 08:38:41AM +0530, Dilip Kumar wrote:
> On Thu, Mar 11, 2021 at 11:55 PM Justin Pryzby wrote:
> > On Wed, Mar 10, 2021 at 08:28:58PM -0600, Justin Pryzby wrote:
> > > This includes a patch to use pkgconfig, in an attempt to build on mac,
> > > which
> > > currently fails
On Mon, Mar 01, 2021 at 08:53:09PM +0530, Dilip Kumar wrote:
> On Mon, Mar 1, 2021 at 5:36 PM Dilip Kumar wrote:
> > On Mon, Mar 1, 2021 at 11:06 AM Justin Pryzby wrote:
> > > Thanks. It seems like that explains it.
> > > I think if that's a problem with recent versions, then you'll have to
> >
On Thu, Mar 11, 2021 at 11:55 PM Justin Pryzby wrote:
>
> On Wed, Mar 10, 2021 at 08:28:58PM -0600, Justin Pryzby wrote:
> > This includes a patch to use pkgconfig, in an attempt to build on mac, which
> > currently fails like:
> >
> >
On Thu, Mar 11, 2021 at 10:07:30AM +0530, Dilip Kumar wrote:
> On Thu, Mar 11, 2021 at 8:50 AM Justin Pryzby wrote:
> >
> > Looking at v23-0002-alter-table-set-compression, ATRewriteTable() was
> > calling
> > CompareCompressionMethodAndDecompress().
>
> While changing the compression method
On Thu, Mar 11, 2021 at 8:50 AM Justin Pryzby wrote:
>
> Looking at v23-0002-alter-table-set-compression, ATRewriteTable() was calling
> CompareCompressionMethodAndDecompress().
While changing the compression method user might be just interested
to compress the future tuple with the new
On Thu, Mar 11, 2021 at 08:17:46AM +0530, Dilip Kumar wrote:
> On Thu, Mar 11, 2021 at 2:21 AM Robert Haas wrote:
> >
> > On Wed, Mar 10, 2021 at 6:52 AM Dilip Kumar wrote:
> > > The pending comment is providing a way to rewrite a table and
> > > re-compress the data with the current compression
On Thu, Mar 11, 2021 at 2:21 AM Robert Haas wrote:
>
> On Wed, Mar 10, 2021 at 6:52 AM Dilip Kumar wrote:
> > The pending comment is providing a way to rewrite a table and
> > re-compress the data with the current compression method.
>
> I spent some time poking at this yesterday and ran
This includes a patch to use pkgconfig, in an attempt to build on mac, which
currently fails like:
https://cirrus-ci.com/task/5993712963551232?command=build#L126
checking for LZ4_compress in -llz4... no
configure: error: library 'lz4' is required for LZ4 support
--
Justin
>From
On Wed, Mar 10, 2021 at 03:50:48PM -0500, Robert Haas wrote:
> On Wed, Mar 10, 2021 at 6:52 AM Dilip Kumar wrote:
> > The pending comment is providing a way to rewrite a table and
> > re-compress the data with the current compression method.
>
> I spent some time poking at this yesterday and ran
On Wed, Mar 10, 2021 at 6:52 AM Dilip Kumar wrote:
> The pending comment is providing a way to rewrite a table and
> re-compress the data with the current compression method.
I spent some time poking at this yesterday and ran couldn't figure out
what was going on here. There are two places where
On Tue, Mar 9, 2021 at 1:56 AM Robert Haas wrote:
>
> With this design, we can support changing the compression method on a
> column quite easily. It's just a hint, like the STORAGE parameter. It
> has no bearing on what can be present in the table, but just controls
> how new values are stored.
On Tue, Mar 9, 2021 at 1:13 PM Justin Pryzby wrote:
>
> On Tue, Mar 09, 2021 at 01:04:10PM +0530, Dilip Kumar wrote:
> > On Tue, Mar 9, 2021 at 2:45 AM Robert Haas wrote:
> > >
> > Yeah, vacuum full or cluster will not re-compress the data. How about
> > providing syntax ALTER TABLE ALTER
On Tue, Mar 09, 2021 at 01:04:10PM +0530, Dilip Kumar wrote:
> On Tue, Mar 9, 2021 at 2:45 AM Robert Haas wrote:
> >
> > On Mon, Mar 8, 2021 at 3:59 PM Justin Pryzby wrote:
> > > > It would be nice to have a way to force
> > > > anything compressed with the old method to be re-compressed with
On Tue, Mar 9, 2021 at 2:45 AM Robert Haas wrote:
>
> On Mon, Mar 8, 2021 at 3:59 PM Justin Pryzby wrote:
> > > It would be nice to have a way to force
> > > anything compressed with the old method to be re-compressed with the
> > > new method, but not having that doesn't preclude allowing the
>
On Mon, Mar 08, 2021 at 03:32:39PM +0530, Dilip Kumar wrote:
> On Sun, Mar 7, 2021 at 1:27 AM Justin Pryzby wrote:
> >
> > On Sat, Mar 06, 2021 at 08:59:16PM +0530, Dilip Kumar wrote:
> > > - Alter table set compression, will not rewrite the old data, so only
> > > the new tuple will be
On Mon, Mar 8, 2021 at 3:59 PM Justin Pryzby wrote:
> > It would be nice to have a way to force
> > anything compressed with the old method to be re-compressed with the
> > new method, but not having that doesn't preclude allowing the
> > parameter to be changed.
>
> Doesn't vacuum
On Mon, Mar 08, 2021 at 03:26:04PM -0500, Robert Haas wrote:
> On Mon, Mar 8, 2021 at 5:02 AM Dilip Kumar wrote:
> > So now only pending point is, how do we handle the upgrade when you
> > are upgrading from --with-lz4 to --without-lz4 binary and a couple of
> > options discussed here are
> > a)
On Mon, Mar 8, 2021 at 5:02 AM Dilip Kumar wrote:
> So now only pending point is, how do we handle the upgrade when you
> are upgrading from --with-lz4 to --without-lz4 binary and a couple of
> options discussed here are
> a) Should we allow table creation with lz4 even if it is compiled
>
On Sun, Mar 07, 2021 at 06:04:41PM +0530, Dilip Kumar wrote:
> On Sun, Mar 7, 2021 at 2:19 PM Justin Pryzby wrote:
> >
> > Earlier in this thread, I suggested to implement an option to pg_restore to
> > avoid outputting compression, in order to allow restoring with a different
> > compression (by
On Sun, Mar 7, 2021 at 2:19 PM Justin Pryzby wrote:
>
> Earlier in this thread, I suggested to implement an option to pg_restore to
> avoid outputting compression, in order to allow restoring with a different
> compression (by using the default_toast_compression GUC). Now, it seems like
> that's
On Sun, Mar 07, 2021 at 01:36:50PM +0530, Dilip Kumar wrote:
> On Sun, Mar 7, 2021 at 12:47 PM Justin Pryzby wrote:
> > > IMHO we can always allow creating the table with lz4 and only error
> > > out when we really need to compress/decompress the data. I like this
> > > behavior because it is
On Sun, Mar 7, 2021 at 12:47 PM Justin Pryzby wrote:
> > IMHO we can always allow creating the table with lz4 and only error
> > out when we really need to compress/decompress the data. I like this
> > behavior because it is the same as libxml. But I am fine with
> > allowing it only in binary
On Sun, Mar 07, 2021 at 12:16:41PM +0530, Dilip Kumar wrote:
> > If I pg_upgrade from an binary with-lz4 to one without-lz4, it fails
> > while restoring the schema, after running check, which is bad:
> > | pg_restore: error: could not execute query: ERROR: not built with lz4
> > support
> >
On Sun, Mar 7, 2021 at 1:27 AM Justin Pryzby wrote:
>
> On Sat, Mar 06, 2021 at 08:59:16PM +0530, Dilip Kumar wrote:
> > - Alter table set compression, will not rewrite the old data, so only
> > the new tuple will be compressed with the new compression method.
> > - No preserve.
>
> +1, this
On Sat, Mar 06, 2021 at 08:59:16PM +0530, Dilip Kumar wrote:
> - Alter table set compression, will not rewrite the old data, so only
> the new tuple will be compressed with the new compression method.
> - No preserve.
+1, this simplifies things. If someone *wants* to rewrite the table, they can
On Thu, Mar 4, 2021 at 2:49 AM Robert Haas wrote:
>
> Hi,
>
> Does this patch need to do something about ExtractReplicaIdentity()?
> If there are compressed fields in the tuple being built, can we rely
> on the decompression engine being available at the time we need to do
> something with the
Hi,
Does this patch need to do something about ExtractReplicaIdentity()?
If there are compressed fields in the tuple being built, can we rely
on the decompression engine being available at the time we need to do
something with the tuple?
More generally, I think it would be good to divide up 0001
On Mon, Mar 1, 2021 at 8:53 PM Dilip Kumar wrote:
>
> Now, I think the only pending thing is related to the expandedrecord,
> basically, currently, we have detoasted the compressed filed only in
> expanded_record_set_field_internal function. I am still not
> completely sure that for the built-in
On Mon, Mar 1, 2021 at 11:06 AM Justin Pryzby wrote:
> Thanks. It seems like that explains it.
> I think if that's a problem with recent versions, then you'll have to
> conditionally disable slicing.
> https://packages.debian.org/liblz4-dev
>
> Slicing isn't generally usable if it sometimes
On Sat, Feb 27, 2021 at 9:35 PM Justin Pryzby wrote:
>
> > Subject: [PATCH v28 3/4] Add default_toast_compression GUC
>
> This part isn't working. My first patch worked somewhat better: due to doing
> strcmp() with the default GUC, it avoided using the cached AM OID. (But it
> would've failed
On Mon, Mar 01, 2021 at 10:32:23AM +0530, Dilip Kumar wrote:
> On Sun, Feb 28, 2021 at 9:48 AM Justin Pryzby wrote:
> >
> > On my PC, this new change is causing a test failure:
> >
> > SELECT SUBSTR(f1, 2000, 50) FROM cmdata1;
> > - substr
> >
On Sun, Feb 28, 2021 at 9:48 AM Justin Pryzby wrote:
>
> On my PC, this new change is causing a test failure:
>
> SELECT SUBSTR(f1, 2000, 50) FROM cmdata1;
> - substr
> -
> -
On my PC, this new change is causing a test failure:
SELECT SUBSTR(f1, 2000, 50) FROM cmdata1;
- substr
-
- 01234567890123456789012345678901234567890123456789
-(1 row)
-
+ERROR: compressed lz4 data
> Subject: [PATCH v28 3/4] Add default_toast_compression GUC
This part isn't working. My first patch worked somewhat better: due to doing
strcmp() with the default GUC, it avoided using the cached AM OID. (But it
would've failed with more than 2 AMs, since the cache wasn't invalidated, since
I
On Fri, Feb 26, 2021 at 8:10 PM Dilip Kumar wrote:
>
> On Sun, Feb 21, 2021 at 5:33 PM Dilip Kumar wrote:
> >
>
> Based on offlist discussion with Robert, I have done further analysis
> of the composite type data. So the Idea is that I have analyzed all
> the callers of
> HeapTupleGetDatum and
On Sat, Feb 20, 2021 at 11:04 AM Dilip Kumar wrote:
>
> On Sat, Feb 20, 2021 at 2:51 AM Robert Haas wrote:
> >
> > On Fri, Feb 19, 2021 at 11:12 AM Dilip Kumar wrote:
> > I think that these performance tests aren't really exercising the
> > expanded-record stuff, just the ExecEvalRow changes.
On Sat, Feb 20, 2021 at 11:04 AM Dilip Kumar wrote:
> Even after the above case, we might say it is still not a problem for
> this patch because even though t2 doesn't have a direct relationship
> with lz4 but it has an indirect relationship with lz4 via t1. So I
> think this particular case
On Sat, Feb 20, 2021 at 2:51 AM Robert Haas wrote:
>
> On Fri, Feb 19, 2021 at 11:12 AM Dilip Kumar wrote:
> I think that these performance tests aren't really exercising the
> expanded-record stuff, just the ExecEvalRow changes. We need to test
> that test case, and I tend to suspect there's
On Fri, Feb 19, 2021 at 4:21 PM Robert Haas wrote:
> What makes me a bit uncomfortable about that approach is that it
> presupposes that everything that uses expanded records has some other
> defense against those tuples getting written to disk without first
> expanding any external datums. And
On Fri, Feb 19, 2021 at 11:12 AM Dilip Kumar wrote:
> I had an off list discussion with Robert and based on his suggestion
> and a poc patch, I have come up with an updated version for handling
> the composite type. Basically, the problem was that ExecEvalRow we
> are first forming the tuple and
On Thu, Feb 11, 2021 at 3:26 AM Robert Haas wrote:
>
> In CompareCompressionMethodAndDecompress, I think this is still
> playing a bit fast and loose with the rules around slots. I think we
> can do better. Suppose that at the point where we discover that we
> need to decompress at least one
On Thu, Feb 11, 2021 at 1:37 AM Robert Haas wrote:
>
> On Wed, Feb 10, 2021 at 9:52 AM Dilip Kumar wrote:
> > [ new patches ]
>
> I think that in both varattrib_4b and toast_internals.h it would be
> better to pick a less generic field name. In toast_internals.h it's
> just info; in postgres.h
On Sat, Feb 13, 2021 at 8:14 PM Dilip Kumar wrote:
>
> On Thu, Feb 11, 2021 at 8:17 PM Robert Haas wrote:
> >
> > On Thu, Feb 11, 2021 at 7:36 AM Dilip Kumar wrote:
> > > W.R.T the attached patch, In HeapTupleHeaderGetDatum, we don't even
> > > attempt to detoast if there is no external field
On Mon, Feb 15, 2021 at 1:58 AM Justin Pryzby wrote:
>
> On Sun, Feb 14, 2021 at 12:49:40PM -0600, Justin Pryzby wrote:
> > On Wed, Feb 10, 2021 at 04:56:17PM -0500, Robert Haas wrote:
> > > Small delta patch with a few other suggested changes attached.
> >
> > Robert's fixup patch caused the CI
On Thu, Feb 11, 2021 at 8:17 PM Robert Haas wrote:
>
> On Thu, Feb 11, 2021 at 7:36 AM Dilip Kumar wrote:
> > W.R.T the attached patch, In HeapTupleHeaderGetDatum, we don't even
> > attempt to detoast if there is no external field in the tuple, in POC
> > I have got rid of that check, but I
On Thu, Feb 11, 2021 at 7:36 AM Dilip Kumar wrote:
> W.R.T the attached patch, In HeapTupleHeaderGetDatum, we don't even
> attempt to detoast if there is no external field in the tuple, in POC
> I have got rid of that check, but I think we might need to do better.
> Maybe we can add a flag in
On Thu, Feb 11, 2021 at 3:26 AM Robert Haas wrote:
>
> On Wed, Feb 10, 2021 at 3:06 PM Robert Haas wrote:
> > I think you have a fairly big problem with row types. Consider this example:
> >
> > create table t1 (a int, b text compression pglz);
> > create table t2 (a int, b text compression
On Wed, Feb 10, 2021 at 3:06 PM Robert Haas wrote:
> I think you have a fairly big problem with row types. Consider this example:
>
> create table t1 (a int, b text compression pglz);
> create table t2 (a int, b text compression lz4);
> create table t3 (x t1);
> insert into t1 values (1,
On Wed, Feb 10, 2021 at 9:52 AM Dilip Kumar wrote:
> [ new patches ]
I think that in both varattrib_4b and toast_internals.h it would be
better to pick a less generic field name. In toast_internals.h it's
just info; in postgres.h it's va_info. But:
[rhaas pgsql]$ git grep info | wc -l
24552
On Wed, Feb 10, 2021 at 1:42 AM Robert Haas wrote:
>
> Please remember to trim unnecessary quoted material.
Okay, I will.
> On Sun, Feb 7, 2021 at 6:45 AM Dilip Kumar wrote:
> > [ a whole lot of quoted stuff ]
> >
> > Conclusion:
> > 1. In most cases lz4 is faster and doing better compression
On Tue, Feb 9, 2021 at 3:37 PM Justin Pryzby wrote:
> I think you misunderstood: I mean that the WIP patch should default to
> --enable-lz4, to exercise on a few CI. It's hardly useful to run CI with the
> feature disabled. I assume that the patch would be committed with default
>
I see the thread got broken somehow (or cfbot thought it did), so I added the
new thread, and this is now passing all tests. (I think using the v22
patches). http://cfbot.cputube.org/dilip-kumar.html
On Fri, Feb 05, 2021 at 11:07:53AM -0500, Robert Haas wrote:
> > Also, I think we may want to
Please remember to trim unnecessary quoted material.
On Sun, Feb 7, 2021 at 6:45 AM Dilip Kumar wrote:
> [ a whole lot of quoted stuff ]
>
> I have tested the performance, pglz vs lz4
>
> Test1: With a small simple string, pglz doesn't select compression but
> lz4 select as no min limit
> Table:
On Tue, Feb 9, 2021 at 2:08 PM Dilip Kumar wrote:
>
> On Sun, Feb 7, 2021 at 5:15 PM Dilip Kumar wrote:
> >
> > On Fri, Feb 5, 2021 at 8:11 PM Dilip Kumar wrote:
> > >
> > > On Wed, Feb 3, 2021 at 2:07 AM Robert Haas wrote:
> > > >
> > > > Even more review comments, still looking mostly at
101 - 200 of 359 matches
Mail list logo
