public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 2/2] default to --with-lz4
2+ messages / 2 participants
[nested] [flat]
* [PATCH 2/2] default to --with-lz4
@ 2021-03-11 23:36 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Justin Pryzby @ 2021-03-11 23:36 UTC (permalink / raw)
this is meant to exercise the new feature in the CIs, and not meant to be
merged
---
configure | 6 ++++--
configure.ac | 4 ++--
src/backend/access/transam/xlog.c | 2 +-
src/backend/utils/misc/guc.c | 4 ++--
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/configure b/configure
index 440d1e8ce5..b411ed1cb4 100755
--- a/configure
+++ b/configure
@@ -1575,7 +1575,7 @@ Optional Packages:
--with-system-tzdata=DIR
use system time zone data in DIR
--without-zlib do not use Zlib
- --with-lz4 build with LZ4 support
+ --without-lz4 build without LZ4 support
--with-gnu-ld assume the C compiler uses GNU ld [default=no]
--with-ssl=LIB use LIB for SSL/TLS support (openssl)
--with-openssl obsolete spelling of --with-ssl=openssl
@@ -8598,7 +8598,9 @@ $as_echo "#define USE_LZ4 1" >>confdefs.h
esac
else
- with_lz4=no
+ with_lz4=yes
+
+$as_echo "#define USE_LZ4 1" >>confdefs.h
fi
diff --git a/configure.ac b/configure.ac
index 780791ae8a..392784d55c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -990,8 +990,8 @@ AC_SUBST(with_zlib)
# LZ4
#
AC_MSG_CHECKING([whether to build with LZ4 support])
-PGAC_ARG_BOOL(with, lz4, no, [build with LZ4 support],
- [AC_DEFINE([USE_LZ4], 1, [Define to 1 to build with LZ4 support. (--with-lz4)])])
+PGAC_ARG_BOOL(with, lz4, yes, [build without LZ4 support],
+ [AC_DEFINE([USE_LZ4], 1, [Define to 1 to build without LZ4 support. (--without-lz4)])])
AC_MSG_RESULT([$with_lz4])
AC_SUBST(with_lz4)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 82cedb0a41..321558a2c6 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -99,7 +99,7 @@ bool EnableHotStandby = false;
bool fullPageWrites = true;
bool wal_log_hints = false;
bool wal_compression = false;
-int wal_compression_method = PGLZ_COMPRESSION_ID;
+int wal_compression_method = LZ4_COMPRESSION_ID;
char *wal_consistency_checking_string = NULL;
bool *wal_consistency_checking = NULL;
bool wal_init_zero = true;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 42253c11f1..6aa14694f4 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1269,7 +1269,7 @@ static struct config_bool ConfigureNamesBool[] =
NULL
},
&wal_compression,
- false,
+ true,
NULL, NULL, NULL
},
@@ -4728,7 +4728,7 @@ static struct config_enum ConfigureNamesEnum[] =
NULL
},
&wal_compression_method,
- PGLZ_COMPRESSION_ID, wal_compression_options,
+ LZ4_COMPRESSION_ID, wal_compression_options,
NULL, NULL, NULL
},
--
2.17.0
--m1UC1K4AOz1Ywdkx--
^ permalink raw reply [nested|flat] 2+ messages in thread
* FDW pushdown of non-collated functions
@ 2023-09-08 14:41 Jean-Christophe Arnu <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Jean-Christophe Arnu @ 2023-09-08 14:41 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>
Dear hackers,
I recently found a weird behaviour involving FDW (postgres_fdw) and
planning.
Here’s a simplified use-case:
Given a remote table (say on server2) with the following definition:
CREATE TABLE t1(
ts timestamp without time zone,
x bigint,
x2 text
);
--Then populate t1 table:INSERT INTO t1
SELECT
current_timestamp - 1000*random()*'1 day'::interval
,x
,''||x
FROM
generate_series(1,100000) as x;
This table is imported in a specific schema on server1 (we do not use
use_remote_estimate) also with t1 name in a specific schema:
On server1:
CREATE SERVER server2
FOREIGN DATA WRAPPER postgres_fdw
OPTIONS (
host '127.0.0.1',
port '9002',
dbname 'postgres',
use_remote_estimate 'false'
);
CREATE USER MAPPING FOR jc
SERVER server2
OPTIONS (user 'jc');
CREATE SCHEMA remote;
IMPORT FOREIGN SCHEMA public
FROM SERVER server2
INTO remote ;
On a classic PostgreSQL 15 version the following query using date_trunc()
is executed and results in the following plan:
jc=# explain (verbose,analyze) select date_trunc('day',ts), count(1)
from remote.t1 group by date_trunc('day',ts) order by 1;
QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------
Sort (cost=216.14..216.64 rows=200 width=16) (actual
time=116.699..116.727 rows=1001 loops=1)
Output: (date_trunc('day'::text, ts)), (count(1))
Sort Key: (date_trunc('day'::text, t1.ts))
Sort Method: quicksort Memory: 79kB
-> HashAggregate (cost=206.00..208.50 rows=200 width=16) (actual
time=116.452..116.532 rows=1001 loops=1)
Output: (date_trunc('day'::text, ts)), count(1)
Group Key: date_trunc('day'::text, t1.ts)
Batches: 1 Memory Usage: 209kB
-> Foreign Scan on remote.t1 (cost=100.00..193.20 rows=2560
width=8) (actual time=0.384..106.225 rows=100000 loops=1)
Output: date_trunc('day'::text, ts)
Remote SQL: SELECT ts FROM public.t1
Planning Time: 0.077 ms
Execution Time: 117.028 ms
Whereas the same query with date_bin()
jc=# explain (verbose,analyze) select
date_bin('1day',ts,'2023-01-01'), count(1) from remote.t1 group by 1
order by 1;
QUERY PLAN
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Foreign Scan (cost=113.44..164.17 rows=200 width=16) (actual
time=11.297..16.312 rows=1001 loops=1)
Output: (date_bin('1 day'::interval, ts, '2023-01-01
00:00:00'::timestamp without time zone)), (count(1))
Relations: Aggregate on (remote.t1)
Remote SQL: SELECT date_bin('1 day'::interval, ts, '2023-01-01
00:00:00'::timestamp without time zone), count(1) FROM public.t1 GROUP
BY 1 ORDER BY date_bin('1 day'::interval, ts, '2023-01-01
00:00:00'::timestamp without time zone) ASC NULLS LAST
Planning Time: 0.114 ms
Execution Time: 16.599 ms
With date_bin() the whole expression is pushed down to the remote server,
whereas with date_trunc() it’s not.
I dived into the code and live debugged. It turns out that decisions to
pushdown or not a whole query depends on many factors like volatility and
collation. In the date_trunc() case, the problem is all about collation (
date_trunc() on timestamp without time zone). And decision is made in the
foreign_expr_walker() in deparse.c (
https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=contrib/postgres_fdw/deparse.c;h=efaf38...
)
First the function is tested as shippable (able to be pushed down) and
date_trunc() and date_bin() both are.
Then parameters sub-expressions are evaluated with collation and
“shippability”, and they all are with both functions.
Then we arrive at this code portion:
if (fe->inputcollid == InvalidOid)
/* OK, inputs are all noncollatable */ ;else if (inner_cxt.state !=
FDW_COLLATE_SAFE ||
fe->inputcollid != inner_cxt.collation)
return false;
For date_trunc() function :
-
fe variable contains the sub-expressions/arguments merged constraints
such as fe->inputcollid. This field is evaluated to 100 (default
collation) so codes jumps to else statement and evaluates the if
predicates. This 100 inputcollationid is due to text predicate 'day'.
-
inner_cxt.state contains FDW_COLLATE_STATE but inner_cxt.collation
contains 0 (InvalidOid) so the control flow returns false thus the
function cannot be pushed down.
For date_bin() function :
- fe variable contains the sub-expressions/arguments merged constraints.
Here, fe->inputcollid is evaluated to 0 (InvalidOid) thus skips the else
statement and continues the control flow in the function.
For date_bin(), all arguments are “non-collatable” arguments (timestamp
without time zone and interval).
So the situation is that date_trunc() is a “non-collatable” function
failing to be pushed down whereas it may be a good idea to do so.
Maybe we could add another condition to the first if statement in order to
allow a “no-collation” function to be pushed down even if they have
“collatable” parameters. I’m not sure about the possible regressions of
behaviour of this change, but it seems to work fine with date_trunc() and
date_part() (which suffers the same problem).
Here’s the following change
/*
* If function's input collation is not derived from a foreign
* Var, it can't be sent to remote.
*/if (fe->inputcollid == InvalidOid ||
fe->funccollid == InvalidOid)
/* OK, inputs are all noncollatable */ ;else if (inner_cxt.state !=
FDW_COLLATE_SAFE ||
fe->inputcollid != inner_cxt.collation)
return false;
I don’t presume this patch is free from side effects or fits all use-cases.
A patch (tiny) is attached to this email. This patch works against
master/head at the time of writing.
Thank you for any thoughts.
--
Jean-Christophe Arnu
Attachments:
[text/x-patch] postgres_fdw_no_collation_function_pushdown.patch (625B, ../../CAHZmTm1Vhu04KAtbYMENt8ahx2pZVimMRYS+hZWADZ6sK-DvMA@mail.gmail.com/3-postgres_fdw_no_collation_function_pushdown.patch)
download | inline diff:
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 09d6dd60dd..16b938ebb2 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -568,7 +568,8 @@ foreign_expr_walker(Node *node,
* If function's input collation is not derived from a foreign
* Var, it can't be sent to remote.
*/
- if (fe->inputcollid == InvalidOid)
+ if (fe->inputcollid == InvalidOid ||
+ fe->funccollid == InvalidOid)
/* OK, inputs are all noncollatable */ ;
else if (inner_cxt.state != FDW_COLLATE_SAFE ||
fe->inputcollid != inner_cxt.collation)
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2023-09-08 14:41 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-11 23:36 [PATCH 2/2] default to --with-lz4 Justin Pryzby <[email protected]>
2023-09-08 14:41 FDW pushdown of non-collated functions Jean-Christophe Arnu <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox