Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit085b6b6

Browse files
committed
Avoid holding a directory FD open across pg_ls_dir_files() calls.
This coding technique is undesirable because (a) it leaks the FD forthe rest of the transaction if the SRF is not run to completion, and(b) allocated FDs are a scarce resource, but multiple interleaveduses of the relevant functions could eat many such FDs.In v11 and later, a query such as "SELECT pg_ls_waldir() LIMIT 1"yields a warning about the leaked FD, and the only reason there'sno warning in earlier branches is that fd.c didn't whine about suchleaks before commit9cb7db3. Even disregarding the warning, itwouldn't be too hard to run a backend out of FDs with careless useof these SQL functions.Hence, rewrite the function so that it reads the directory withina single call, returning the results as a tuplestore rather thanvia value-per-call mode.There are half a dozen other built-in SRFs with similar problems,but let's fix this one to start with, just to see if the buildfarmfinds anything wrong with the code.In passing, fix bogus error report for stat() failure: it waswhining about the directory when it should be fingering theindividual file. Doubtless a copy-and-paste error.Back-patch to v10 where this function was added.Justin Pryzby, with cosmetic tweaks and test cases by meDiscussion:https://postgr.es/m/20200308173103.GC1357@telsasoft.com
1 parentbf68b79 commit085b6b6

File tree

3 files changed

+118
-45
lines changed

3 files changed

+118
-45
lines changed

‎src/backend/utils/adt/genfile.c

Lines changed: 52 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -523,75 +523,82 @@ pg_ls_dir_1arg(PG_FUNCTION_ARGS)
523523
returnpg_ls_dir(fcinfo);
524524
}
525525

526-
/* Generic function to return a directory listing of files */
526+
/*
527+
* Generic function to return a directory listing of files.
528+
*
529+
* If the directory isn't there, silently return an empty set if missing_ok.
530+
* Other unreadable-directory cases throw an error.
531+
*/
527532
staticDatum
528533
pg_ls_dir_files(FunctionCallInfofcinfo,constchar*dir,boolmissing_ok)
529534
{
530-
FuncCallContext*funcctx;
535+
ReturnSetInfo*rsinfo= (ReturnSetInfo*)fcinfo->resultinfo;
536+
boolrandomAccess;
537+
TupleDesctupdesc;
538+
Tuplestorestate*tupstore;
539+
DIR*dirdesc;
531540
structdirent*de;
532-
directory_fctx*fctx;
541+
MemoryContextoldcontext;
533542

534-
if (SRF_IS_FIRSTCALL())
535-
{
536-
MemoryContextoldcontext;
537-
TupleDesctupdesc;
543+
/* check to see if caller supports us returning a tuplestore */
544+
if (rsinfo==NULL|| !IsA(rsinfo,ReturnSetInfo))
545+
ereport(ERROR,
546+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
547+
errmsg("set-valued function called in context that cannot accept a set")));
548+
if (!(rsinfo->allowedModes&SFRM_Materialize))
549+
ereport(ERROR,
550+
(errcode(ERRCODE_SYNTAX_ERROR),
551+
errmsg("materialize mode required, but it is not "
552+
"allowed in this context")));
538553

539-
funcctx=SRF_FIRSTCALL_INIT();
540-
oldcontext=MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
554+
/* The tupdesc and tuplestore must be created in ecxt_per_query_memory */
555+
oldcontext=MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
541556

542-
fctx=palloc(sizeof(directory_fctx));
557+
if (get_call_result_type(fcinfo,NULL,&tupdesc)!=TYPEFUNC_COMPOSITE)
558+
elog(ERROR,"return type must be a row type");
543559

544-
tupdesc=CreateTemplateTupleDesc(3);
545-
TupleDescInitEntry(tupdesc, (AttrNumber)1,"name",
546-
TEXTOID,-1,0);
547-
TupleDescInitEntry(tupdesc, (AttrNumber)2,"size",
548-
INT8OID,-1,0);
549-
TupleDescInitEntry(tupdesc, (AttrNumber)3,"modification",
550-
TIMESTAMPTZOID,-1,0);
551-
funcctx->tuple_desc=BlessTupleDesc(tupdesc);
560+
randomAccess= (rsinfo->allowedModes&SFRM_Materialize_Random)!=0;
561+
tupstore=tuplestore_begin_heap(randomAccess, false,work_mem);
562+
rsinfo->returnMode=SFRM_Materialize;
563+
rsinfo->setResult=tupstore;
564+
rsinfo->setDesc=tupdesc;
552565

553-
fctx->location=pstrdup(dir);
554-
fctx->dirdesc=AllocateDir(fctx->location);
566+
MemoryContextSwitchTo(oldcontext);
555567

556-
if (!fctx->dirdesc)
568+
/*
569+
* Now walk the directory. Note that we must do this within a single SRF
570+
* call, not leave the directory open across multiple calls, since we
571+
* can't count on the SRF being run to completion.
572+
*/
573+
dirdesc=AllocateDir(dir);
574+
if (!dirdesc)
575+
{
576+
/* Return empty tuplestore if appropriate */
577+
if (missing_ok&&errno==ENOENT)
557578
{
558-
if (missing_ok&&errno==ENOENT)
559-
{
560-
MemoryContextSwitchTo(oldcontext);
561-
SRF_RETURN_DONE(funcctx);
562-
}
563-
else
564-
ereport(ERROR,
565-
(errcode_for_file_access(),
566-
errmsg("could not open directory \"%s\": %m",
567-
fctx->location)));
579+
tuplestore_donestoring(tupstore);
580+
return (Datum)0;
568581
}
569-
570-
funcctx->user_fctx=fctx;
571-
MemoryContextSwitchTo(oldcontext);
582+
/* Otherwise, we can let ReadDir() throw the error */
572583
}
573584

574-
funcctx=SRF_PERCALL_SETUP();
575-
fctx= (directory_fctx*)funcctx->user_fctx;
576-
577-
while ((de=ReadDir(fctx->dirdesc,fctx->location))!=NULL)
585+
while ((de=ReadDir(dirdesc,dir))!=NULL)
578586
{
579587
Datumvalues[3];
580588
boolnulls[3];
581589
charpath[MAXPGPATH*2];
582590
structstatattrib;
583-
HeapTupletuple;
584591

585592
/* Skip hidden files */
586593
if (de->d_name[0]=='.')
587594
continue;
588595

589596
/* Get the file info */
590-
snprintf(path,sizeof(path),"%s/%s",fctx->location,de->d_name);
597+
snprintf(path,sizeof(path),"%s/%s",dir,de->d_name);
591598
if (stat(path,&attrib)<0)
592599
ereport(ERROR,
593600
(errcode_for_file_access(),
594-
errmsg("could not statdirectory \"%s\": %m",dir)));
601+
errmsg("could not statfile \"%s\": %m",path)));
595602

596603
/* Ignore anything but regular files */
597604
if (!S_ISREG(attrib.st_mode))
@@ -602,12 +609,12 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok)
602609
values[2]=TimestampTzGetDatum(time_t_to_timestamptz(attrib.st_mtime));
603610
memset(nulls,0,sizeof(nulls));
604611

605-
tuple=heap_form_tuple(funcctx->tuple_desc,values,nulls);
606-
SRF_RETURN_NEXT(funcctx,HeapTupleGetDatum(tuple));
612+
tuplestore_putvalues(tupstore,tupdesc,values,nulls);
607613
}
608614

609-
FreeDir(fctx->dirdesc);
610-
SRF_RETURN_DONE(funcctx);
615+
FreeDir(dirdesc);
616+
tuplestore_donestoring(tupstore);
617+
return (Datum)0;
611618
}
612619

613620
/* Function to return the list of files in the log directory */

‎src/test/regress/expected/misc_functions.out

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,52 @@ ERROR: function num_nulls() does not exist
133133
LINE 1: SELECT num_nulls();
134134
^
135135
HINT: No function matches the given name and argument types. You might need to add explicit type casts.
136+
--
137+
-- Test some built-in SRFs
138+
--
139+
-- The outputs of these are variable, so we can't just print their results
140+
-- directly, but we can at least verify that the code doesn't fail.
141+
--
142+
select setting as segsize
143+
from pg_settings where name = 'wal_segment_size'
144+
\gset
145+
select count(*) > 0 as ok from pg_ls_waldir();
146+
ok
147+
----
148+
t
149+
(1 row)
150+
151+
-- Test ProjectSet as well as FunctionScan
152+
select count(*) > 0 as ok from (select pg_ls_waldir()) ss;
153+
ok
154+
----
155+
t
156+
(1 row)
157+
158+
-- Test not-run-to-completion cases.
159+
select * from pg_ls_waldir() limit 0;
160+
name | size | modification
161+
------+------+--------------
162+
(0 rows)
163+
164+
select count(*) > 0 as ok from (select * from pg_ls_waldir() limit 1) ss;
165+
ok
166+
----
167+
t
168+
(1 row)
169+
170+
select (pg_ls_waldir()).size = :segsize as ok limit 1;
171+
ok
172+
----
173+
t
174+
(1 row)
175+
176+
select count(*) >= 0 as ok from pg_ls_archive_statusdir();
177+
ok
178+
----
179+
t
180+
(1 row)
181+
136182
--
137183
-- Test adding a support function to a subject function
138184
--

‎src/test/regress/sql/misc_functions.sql

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,26 @@ SELECT num_nulls(VARIADIC '{}'::int[]);
3030
SELECT num_nonnulls();
3131
SELECT num_nulls();
3232

33+
--
34+
-- Test some built-in SRFs
35+
--
36+
-- The outputs of these are variable, so we can't just print their results
37+
-- directly, but we can at least verify that the code doesn't fail.
38+
--
39+
select settingas segsize
40+
from pg_settingswhere name='wal_segment_size'
41+
\gset
42+
43+
selectcount(*)>0as okfrom pg_ls_waldir();
44+
-- Test ProjectSet as well as FunctionScan
45+
selectcount(*)>0as okfrom (select pg_ls_waldir()) ss;
46+
-- Test not-run-to-completion cases.
47+
select*from pg_ls_waldir()limit0;
48+
selectcount(*)>0as okfrom (select*from pg_ls_waldir()limit1) ss;
49+
select (pg_ls_waldir()).size= :segsizeas oklimit1;
50+
51+
selectcount(*)>=0as okfrom pg_ls_archive_statusdir();
52+
3353
--
3454
-- Test adding a support function to a subject function
3555
--

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp