public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v17 06/10] pg_ls_*dir to show directories and "isdir" column..
5+ messages / 3 participants
[nested] [flat]
* [PATCH v17 06/10] pg_ls_*dir to show directories and "isdir" column..
@ 2020-03-09 06:00 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Justin Pryzby @ 2020-03-09 06:00 UTC (permalink / raw)
pg_ls_logdir, pg_ls_waldir, pg_ls_archive_statusdir
Need catversion bump
---
doc/src/sgml/func.sgml | 38 +++++++++++---------
src/backend/utils/adt/genfile.c | 11 +++---
src/include/catalog/pg_proc.dat | 12 +++----
src/test/regress/expected/misc_functions.out | 4 +--
4 files changed, 37 insertions(+), 28 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index c9d1857182..8e0de7c02d 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -25700,12 +25700,14 @@ SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size
<returnvalue>setof record</returnvalue>
( <parameter>name</parameter> <type>text</type>,
<parameter>size</parameter> <type>bigint</type>,
- <parameter>modification</parameter> <type>timestamp with time zone</type> )
+ <parameter>modification</parameter> <type>timestamp with time zone</type>,
+ <parameter>isdir</parameter> <type>boolean</type> )
</para>
<para>
- Returns the name, size, and last modification time (mtime) of each
- ordinary file in the server's log directory. Filenames beginning with
- a dot, directories, and other special files are excluded.
+ For each file in the server's log directory,
+ return the file's name, size, last modification time (mtime), and a boolean
+ indicating if the file is a directory.
+ Filenames beginning with a dot and special file types are excluded.
</para>
<para>
This function is restricted to superusers and members of
@@ -25725,7 +25727,8 @@ SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size
<returnvalue>setof record</returnvalue>
( <parameter>name</parameter> <type>text</type>,
<parameter>size</parameter> <type>bigint</type>,
- <parameter>modification</parameter> <type>timestamp with time zone</type> )
+ <parameter>modification</parameter> <type>timestamp with time zone</type>,
+ <parameter>isdir</parameter> <type>boolean</type> )
</para>
<para>
For each file in the specified directory, list the file and its
@@ -25744,13 +25747,14 @@ SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size
<returnvalue>setof record</returnvalue>
( <parameter>name</parameter> <type>text</type>,
<parameter>size</parameter> <type>bigint</type>,
- <parameter>modification</parameter> <type>timestamp with time zone</type> )
+ <parameter>modification</parameter> <type>timestamp with time zone</type>,
+ <parameter>isdir</parameter> <type>boolean</type> )
</para>
<para>
- Returns the name, size, and last modification time (mtime) of each
- ordinary file in the server's write-ahead log (WAL) directory.
- Filenames beginning with a dot, directories, and other special files
- are excluded.
+ For each file in the server's write-ahead log (WAL) directory, list the
+ file's name, size, last modification time (mtime), and a boolean
+ indicating if the file is a directory.
+ Filenames beginning with a dot and special files types are excluded.
</para>
<para>
This function is restricted to superusers and members of
@@ -25768,13 +25772,15 @@ SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size
<returnvalue>setof record</returnvalue>
( <parameter>name</parameter> <type>text</type>,
<parameter>size</parameter> <type>bigint</type>,
- <parameter>modification</parameter> <type>timestamp with time zone</type> )
+ <parameter>modification</parameter> <type>timestamp with time zone</type>,
+ <parameter>isdir</parameter> <type>boolean</type> )
</para>
<para>
- Returns the name, size, and last modification time (mtime) of each
- ordinary file in the server's WAL archive status directory
- (<filename>pg_wal/archive_status</filename>). Filenames beginning
- with a dot, directories, and other special files are excluded.
+ For each file in the server's WAL archive status directory
+ (<filename>pg_wal/archive_status</filename>), list the file's
+ name, size, last modification time (mtime), and a boolean indicating if
+ the file is a directory.
+ Filenames beginning with a dot and special file types are excluded.
</para>
<para>
This function is restricted to superusers and members of
@@ -25798,7 +25804,7 @@ SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size
</para>
<para>
For each file in the temporary directory within the given
- <parameter>tablespace</parameter>, return each file's name, size, last
+ <parameter>tablespace</parameter>, list the file's name, size, last
modification time (mtime) and a boolean indicating if the file is a directory.
Directories are used for temporary files shared by parallel processes.
If <parameter>tablespace</parameter> is not provided, the
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 023f1ad605..fba63568b6 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -52,6 +52,9 @@ static Datum pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, int flags
*/
#define LS_DIR_HISTORIC (LS_DIR_SKIP_DIRS|LS_DIR_SKIP_HIDDEN|LS_DIR_SKIP_SPECIAL|LS_DIR_METADATA)
+/* Shortcut for common behavior */
+#define LS_DIR_COMMON (LS_DIR_SKIP_HIDDEN|LS_DIR_SKIP_SPECIAL|LS_DIR_METADATA)
+
/*
* Convert a "text" filename argument to C string, and check it's allowable.
*
@@ -644,14 +647,14 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, int flags)
Datum
pg_ls_logdir(PG_FUNCTION_ARGS)
{
- return pg_ls_dir_files(fcinfo, Log_directory, LS_DIR_HISTORIC);
+ return pg_ls_dir_files(fcinfo, Log_directory, LS_DIR_COMMON);
}
/* Function to return the list of files in the WAL directory */
Datum
pg_ls_waldir(PG_FUNCTION_ARGS)
{
- return pg_ls_dir_files(fcinfo, XLOGDIR, LS_DIR_HISTORIC);
+ return pg_ls_dir_files(fcinfo, XLOGDIR, LS_DIR_COMMON);
}
/*
@@ -670,7 +673,7 @@ pg_ls_tmpdir(FunctionCallInfo fcinfo, Oid tblspc)
TempTablespacePath(path, tblspc);
return pg_ls_dir_files(fcinfo, path,
- LS_DIR_SKIP_HIDDEN | LS_DIR_SKIP_SPECIAL | LS_DIR_ISDIR | LS_DIR_METADATA | LS_DIR_MISSING_OK);
+ LS_DIR_COMMON | LS_DIR_MISSING_OK);
}
/*
@@ -700,7 +703,7 @@ Datum
pg_ls_archive_statusdir(PG_FUNCTION_ARGS)
{
return pg_ls_dir_files(fcinfo, XLOGDIR "/archive_status",
- LS_DIR_HISTORIC | LS_DIR_MISSING_OK);
+ LS_DIR_COMMON | LS_DIR_MISSING_OK);
}
/*
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index dc3b497587..adfce45d1a 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10880,18 +10880,18 @@
{ oid => '3353', descr => 'list files in the log directory',
proname => 'pg_ls_logdir', procost => '10', prorows => '20', proretset => 't',
provolatile => 'v', prorettype => 'record', proargtypes => '',
- proallargtypes => '{text,int8,timestamptz}', proargmodes => '{o,o,o}',
- proargnames => '{name,size,modification}', prosrc => 'pg_ls_logdir' },
+ proallargtypes => '{text,int8,timestamptz,bool}', proargmodes => '{o,o,o,o}',
+ proargnames => '{name,size,modification,isdir}', prosrc => 'pg_ls_logdir' },
{ oid => '3354', descr => 'list of files in the WAL directory',
proname => 'pg_ls_waldir', procost => '10', prorows => '20', proretset => 't',
provolatile => 'v', prorettype => 'record', proargtypes => '',
- proallargtypes => '{text,int8,timestamptz}', proargmodes => '{o,o,o}',
- proargnames => '{name,size,modification}', prosrc => 'pg_ls_waldir' },
+ proallargtypes => '{text,int8,timestamptz,bool}', proargmodes => '{o,o,o,o}',
+ proargnames => '{name,size,modification,isdir}', prosrc => 'pg_ls_waldir' },
{ oid => '5031', descr => 'list of files in the archive_status directory',
proname => 'pg_ls_archive_statusdir', procost => '10', prorows => '20',
proretset => 't', provolatile => 'v', prorettype => 'record',
- proargtypes => '', proallargtypes => '{text,int8,timestamptz}',
- proargmodes => '{o,o,o}', proargnames => '{name,size,modification}',
+ proargtypes => '', proallargtypes => '{text,int8,timestamptz,bool}',
+ proargmodes => '{o,o,o,o}', proargnames => '{name,size,modification,isdir}',
prosrc => 'pg_ls_archive_statusdir' },
{ oid => '5029', descr => 'list files in the pgsql_tmp directory',
proname => 'pg_ls_tmpdir', procost => '10', prorows => '20', proretset => 't',
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index 38493de732..64b1417fb8 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -157,8 +157,8 @@ select count(*) > 0 as ok from (select pg_ls_waldir()) ss;
-- Test not-run-to-completion cases.
select * from pg_ls_waldir() limit 0;
- name | size | modification
-------+------+--------------
+ name | size | modification | isdir
+------+------+--------------+-------
(0 rows)
select count(*) > 0 as ok from (select * from pg_ls_waldir() limit 1) ss;
--
2.17.0
--4LFBTxd4L5NLO6ly
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v17-0007-Add-pg_ls_dir_recurse-to-show-dir-recursively.patch"
^ permalink raw reply [nested|flat] 5+ messages in thread
* Index-only scans vs. partially-retrievable indexes
@ 2022-01-02 19:14 Tom Lane <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Tom Lane @ 2022-01-02 19:14 UTC (permalink / raw)
To: [email protected]
Yesterday I pushed what I thought was a quick fix for bug #17350 [1].
In short, if we have an index that stores both "x" and "f(x)",
where the "x" column can be retrieved in index-only scans but "f(x)"
cannot, it's possible for the planner to generate an IOS plan that
nonetheless tries to read the f(x) index column. The bug report
concerns the case where f(x) is needed in the IOS plan node's targetlist,
and I did fix that --- but I now realize that we still have a problem
with respect to rechecks of the plan node's indexquals. Here's
an example:
regression=# create extension pg_trgm;
CREATE EXTENSION
regression=# create table t(a text);
CREATE TABLE
regression=# create index on t using gist(lower(a) gist_trgm_ops) include (a);
CREATE INDEX
regression=# insert into t values('zed');
INSERT 0 1
regression=# insert into t values('z');
INSERT 0 1
regression=# select * from t where lower(a) like 'z';
a
---
z
(1 row)
That's the correct answer, but we're using a bitmap scan to get it.
If we force an IOS plan:
regression=# set enable_bitmapscan = 0;
SET
regression=# explain select * from t where lower(a) like 'z';
QUERY PLAN
------------------------------------------------------------------------------
Index Only Scan using t_lower_a_idx on t (cost=0.14..28.27 rows=7 width=32)
Index Cond: ((lower(a)) ~~ 'z'::text)
(2 rows)
regression=# select * from t where lower(a) like 'z';
a
---
(0 rows)
That's from a build a few days old. As of HEAD it's even worse;
not only do we fail to return the rows we should, but EXPLAIN says
regression=# explain select * from t where lower(a) like 'z';
QUERY PLAN
------------------------------------------------------------------------------
Index Only Scan using t_lower_a_idx on t (cost=0.14..28.27 rows=7 width=32)
Index Cond: ((NULL::text) ~~ 'z'::text)
(2 rows)
At least this is showing us what's happening: the index recheck condition
sees a NULL for the value of lower(a). That's because it's trying to
get the value of lower(a) out of the index, instead of recomputing it
from the value of a.
AFAICS this has been broken since 9.5 allowed indexes to contain
both retrievable and non-retrievable columns, so it's a bit surprising
that it hasn't been reported before. I suppose that the case was
harder to hit before we introduced INCLUDE columns. The relevant
code actually claims that it's impossible:
/*
* If the index was lossy, we have to recheck the index quals.
* (Currently, this can never happen, but we should support the case
* for possible future use, eg with GiST indexes.)
*/
if (scandesc->xs_recheck)
{
econtext->ecxt_scantuple = slot;
if (!ExecQualAndReset(node->indexqual, econtext))
{
/* Fails recheck, so drop it and loop back for another */
InstrCountFiltered2(node, 1);
continue;
}
}
That comment may have been true when written (it dates to 9.2) but
it's demonstrably not true now; the test case I just gave traverses
this code, and gets the wrong answer.
I don't think there is any way to fix this that doesn't involve
adding another field to structs IndexOnlyScan and IndexOnlyScanState.
We need a version of the indexqual that references the retrievable
index column x and computes f(x) from that, but the indexqual that's
passed to the index AM still has to reference the f(x) index column.
That's annoying from an API stability standpoint. In the back
branches, we can add the new fields at the end to minimize ABI
breakage, but we will still be breaking any extension code that thinks
it knows how to generate an IndexOnlyScan node directly. (But maybe
there isn't any. The Path representation doesn't need to change, so
typical planner extensions should be OK.)
Unless somebody's got a better idea, I'll push forward with making
this happen.
regards, tom lane
[1] https://www.postgresql.org/message-id/flat/17350-b5bdcf476e5badbb%40postgresql.org
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Index-only scans vs. partially-retrievable indexes
@ 2022-01-03 11:13 Andrey Borodin <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Andrey Borodin @ 2022-01-03 11:13 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: [email protected]
> regression=# explain select * from t where lower(a) like 'z';
> QUERY PLAN
> ------------------------------------------------------------------------------
> Index Only Scan using t_lower_a_idx on t (cost=0.14..28.27 rows=7 width=32)
> Index Cond: ((lower(a)) ~~ 'z'::text)
> (2 rows)
>
I've tried to toy with the patch and remembered one related caveat.
If we have index for both returnable and nonreturnable attributes, IOS will not be choosen:
postgres=# create index on t using gist(a gist_trgm_ops) include (a);
postgres=# explain select * from t where a like 'z';
QUERY PLAN
---------------------------------------------------------------------
Index Scan using t_a_a1_idx on t (cost=0.12..8.14 rows=1 width=32)
Index Cond: (a ~~ 'z'::text)
(2 rows)
But with index
create index on t using gist(lower(a) gist_trgm_ops) include (a);
I observe IOS for
select * from t where lower(a) like 'z';
So lossiness of opclass kind of "defeats" returnable attribute. But lossiness of expression does not. I don't feel condifent in surrounding code to say is it a bug or just a lack of feature. But maybe we would like to have equal behavior in both cases...
Thanks!
Best regards, Andrey Borodin.
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Index-only scans vs. partially-retrievable indexes
@ 2022-01-03 14:57 Tom Lane <[email protected]>
parent: Andrey Borodin <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Tom Lane @ 2022-01-03 14:57 UTC (permalink / raw)
To: Andrey Borodin <[email protected]>; +Cc: [email protected]
Andrey Borodin <[email protected]> writes:
> I've tried to toy with the patch and remembered one related caveat.
> If we have index for both returnable and nonreturnable attributes, IOS will not be choosen:
> postgres=# create index on t using gist(a gist_trgm_ops) include (a);
> postgres=# explain select * from t where a like 'z';
> QUERY PLAN
> ---------------------------------------------------------------------
> Index Scan using t_a_a1_idx on t (cost=0.12..8.14 rows=1 width=32)
> Index Cond: (a ~~ 'z'::text)
> (2 rows)
This case is improved by 0002, no?
regards, tom lane
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Index-only scans vs. partially-retrievable indexes
@ 2022-01-03 16:36 Andrey Borodin <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Andrey Borodin @ 2022-01-03 16:36 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: [email protected]
> Andrey Borodin <[email protected]> writes:
>
>> I've tried to toy with the patch and remembered one related caveat.
>> If we have index for both returnable and nonreturnable attributes, IOS will not be choosen:
>
>> postgres=# create index on t using gist(a gist_trgm_ops) include (a);
>> postgres=# explain select * from t where a like 'z';
>> QUERY PLAN
>> ---------------------------------------------------------------------
>> Index Scan using t_a_a1_idx on t (cost=0.12..8.14 rows=1 width=32)
>> Index Cond: (a ~~ 'z'::text)
>> (2 rows)
>
> This case is improved by 0002, no?
>
Uhmm, yes, you are right. Works as expected with the second patch.
I tried first patch against this before writing. But did not expect much from a revert...
Thanks you!
Best regards, Andrey Borodin.
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2022-01-03 16:36 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-03-09 06:00 [PATCH v17 06/10] pg_ls_*dir to show directories and "isdir" column.. Justin Pryzby <[email protected]>
2022-01-02 19:14 Index-only scans vs. partially-retrievable indexes Tom Lane <[email protected]>
2022-01-03 11:13 ` Re: Index-only scans vs. partially-retrievable indexes Andrey Borodin <[email protected]>
2022-01-03 14:57 ` Re: Index-only scans vs. partially-retrievable indexes Tom Lane <[email protected]>
2022-01-03 16:36 ` Re: Index-only scans vs. partially-retrievable indexes Andrey Borodin <[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