Re: Re: A separate table level option to control compression

On Tue, Apr 02, 2019 at 11:37:56AM +0900, Masahiko Sawada wrote:
> Marked.

+ compress_tuple_threshold = RelationGetCompressTupleTarget(relation,
+ toast_tuple_threshold);
+ compress_tuple_threshold = Min(compress_tuple_threshold,
+ toast_tuple_threshold);
All the callers of RelationGetCompressTupleTarget directly compile the
minimum between compress_tuple_threshold and toast_tuple_threshold,
and also call RelationGetToastTupleTarget() beforehand. Wouldn't it
be better to merge all that in a common routine? The same calculation
method is duplicated 5 times.

+ /*
+ * Get the limit at which we should apply compression. This will be same as
+ * maxDataLen unless overridden by the user explicitly.
+ */
+ maxCompressLen = RelationGetCompressTupleTarget(rel, maxDataLen) - hoff;

Is that fine? hoff gets substracted twice.

+ This parameter cannot be set for TOAST tables.
Missing <acronym> markup here for TOAST.

The format of the whole new documentation paragraph is a bit weird.
Could it be possible to adjust it 72-80 characters per line?

The same boundary logic applies to compress_tuple_target and
toast_tuple_target. Perhaps it would make sense to remove the 4-tuple
limit-per-block from the new paragraph, and mention that the same
limits as for toast_tuple_target apply to compress_tuple_target.

+-- expect 0 blocks
+select 0 = pg_relation_size('pg_toast.pg_toast_'||(select oid from
pg_class where relname =
'compresstest250'))/current_setting('block_size')::integer as blocks;
+select 0 = pg_relation_size('pg_toast.pg_toast_'||(select oid from
pg_class where relname =
'compresstest2040'))/current_setting('block_size')::integer as blocks;

This is unreadable. Cannot pg_class.reltoastrelid be used instead?
--
Michael