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

Commit3c8864f

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 parentb7739eb commit3c8864f

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
@@ -522,75 +522,82 @@ pg_ls_dir_1arg(PG_FUNCTION_ARGS)
522522
returnpg_ls_dir(fcinfo);
523523
}
524524

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

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

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

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

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

552-
fctx->location=pstrdup(dir);
553-
fctx->dirdesc=AllocateDir(fctx->location);
565+
MemoryContextSwitchTo(oldcontext);
554566

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

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

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

588595
/* Get the file info */
589-
snprintf(path,sizeof(path),"%s/%s",fctx->location,de->d_name);
596+
snprintf(path,sizeof(path),"%s/%s",dir,de->d_name);
590597
if (stat(path,&attrib)<0)
591598
ereport(ERROR,
592599
(errcode_for_file_access(),
593-
errmsg("could not statdirectory \"%s\": %m",dir)));
600+
errmsg("could not statfile \"%s\": %m",path)));
594601

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

604-
tuple=heap_form_tuple(funcctx->tuple_desc,values,nulls);
605-
SRF_RETURN_NEXT(funcctx,HeapTupleGetDatum(tuple));
611+
tuplestore_putvalues(tupstore,tupdesc,values,nulls);
606612
}
607613

608-
FreeDir(fctx->dirdesc);
609-
SRF_RETURN_DONE(funcctx);
614+
FreeDir(dirdesc);
615+
tuplestore_donestoring(tupstore);
616+
return (Datum)0;
610617
}
611618

612619
/* 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