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

Commit63b51df

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 parent7c094a1 commit63b51df

File tree

3 files changed

+100
-41
lines changed

3 files changed

+100
-41
lines changed

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

Lines changed: 42 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -518,67 +518,68 @@ pg_ls_dir_1arg(PG_FUNCTION_ARGS)
518518
returnpg_ls_dir(fcinfo);
519519
}
520520

521-
/* Generic function to return a directory listing of files */
521+
/*
522+
* Generic function to return a directory listing of files.
523+
*/
522524
staticDatum
523525
pg_ls_dir_files(FunctionCallInfofcinfo,constchar*dir)
524526
{
525-
FuncCallContext*funcctx;
527+
ReturnSetInfo*rsinfo= (ReturnSetInfo*)fcinfo->resultinfo;
528+
boolrandomAccess;
529+
TupleDesctupdesc;
530+
Tuplestorestate*tupstore;
531+
DIR*dirdesc;
526532
structdirent*de;
527-
directory_fctx*fctx;
528-
529-
if (SRF_IS_FIRSTCALL())
530-
{
531-
MemoryContextoldcontext;
532-
TupleDesctupdesc;
533-
534-
funcctx=SRF_FIRSTCALL_INIT();
535-
oldcontext=MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
536-
537-
fctx=palloc(sizeof(directory_fctx));
533+
MemoryContextoldcontext;
538534

539-
tupdesc=CreateTemplateTupleDesc(3, false);
540-
TupleDescInitEntry(tupdesc, (AttrNumber)1,"name",
541-
TEXTOID,-1,0);
542-
TupleDescInitEntry(tupdesc, (AttrNumber)2,"size",
543-
INT8OID,-1,0);
544-
TupleDescInitEntry(tupdesc, (AttrNumber)3,"modification",
545-
TIMESTAMPTZOID,-1,0);
546-
funcctx->tuple_desc=BlessTupleDesc(tupdesc);
535+
/* check to see if caller supports us returning a tuplestore */
536+
if (rsinfo==NULL|| !IsA(rsinfo,ReturnSetInfo))
537+
ereport(ERROR,
538+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
539+
errmsg("set-valued function called in context that cannot accept a set")));
540+
if (!(rsinfo->allowedModes&SFRM_Materialize))
541+
ereport(ERROR,
542+
(errcode(ERRCODE_SYNTAX_ERROR),
543+
errmsg("materialize mode required, but it is not "
544+
"allowed in this context")));
547545

548-
fctx->location=pstrdup(dir);
549-
fctx->dirdesc=AllocateDir(fctx->location);
546+
/* The tupdesc and tuplestore must be created in ecxt_per_query_memory */
547+
oldcontext=MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
550548

551-
if (!fctx->dirdesc)
552-
ereport(ERROR,
553-
(errcode_for_file_access(),
554-
errmsg("could not open directory \"%s\": %m",
555-
fctx->location)));
549+
if (get_call_result_type(fcinfo,NULL,&tupdesc)!=TYPEFUNC_COMPOSITE)
550+
elog(ERROR,"return type must be a row type");
556551

557-
funcctx->user_fctx=fctx;
558-
MemoryContextSwitchTo(oldcontext);
559-
}
552+
randomAccess= (rsinfo->allowedModes&SFRM_Materialize_Random)!=0;
553+
tupstore=tuplestore_begin_heap(randomAccess, false,work_mem);
554+
rsinfo->returnMode=SFRM_Materialize;
555+
rsinfo->setResult=tupstore;
556+
rsinfo->setDesc=tupdesc;
560557

561-
funcctx=SRF_PERCALL_SETUP();
562-
fctx= (directory_fctx*)funcctx->user_fctx;
558+
MemoryContextSwitchTo(oldcontext);
563559

564-
while ((de=ReadDir(fctx->dirdesc,fctx->location))!=NULL)
560+
/*
561+
* Now walk the directory. Note that we must do this within a single SRF
562+
* call, not leave the directory open across multiple calls, since we
563+
* can't count on the SRF being run to completion.
564+
*/
565+
dirdesc=AllocateDir(dir);
566+
while ((de=ReadDir(dirdesc,dir))!=NULL)
565567
{
566568
Datumvalues[3];
567569
boolnulls[3];
568570
charpath[MAXPGPATH*2];
569571
structstatattrib;
570-
HeapTupletuple;
571572

572573
/* Skip hidden files */
573574
if (de->d_name[0]=='.')
574575
continue;
575576

576577
/* Get the file info */
577-
snprintf(path,sizeof(path),"%s/%s",fctx->location,de->d_name);
578+
snprintf(path,sizeof(path),"%s/%s",dir,de->d_name);
578579
if (stat(path,&attrib)<0)
579580
ereport(ERROR,
580581
(errcode_for_file_access(),
581-
errmsg("could not statdirectory \"%s\": %m",dir)));
582+
errmsg("could not statfile \"%s\": %m",path)));
582583

583584
/* Ignore anything but regular files */
584585
if (!S_ISREG(attrib.st_mode))
@@ -589,12 +590,12 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir)
589590
values[2]=TimestampTzGetDatum(time_t_to_timestamptz(attrib.st_mtime));
590591
memset(nulls,0,sizeof(nulls));
591592

592-
tuple=heap_form_tuple(funcctx->tuple_desc,values,nulls);
593-
SRF_RETURN_NEXT(funcctx,HeapTupleGetDatum(tuple));
593+
tuplestore_putvalues(tupstore,tupdesc,values,nulls);
594594
}
595595

596-
FreeDir(fctx->dirdesc);
597-
SRF_RETURN_DONE(funcctx);
596+
FreeDir(dirdesc);
597+
tuplestore_donestoring(tupstore);
598+
return (Datum)0;
598599
}
599600

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

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,3 +133,43 @@ 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+

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,21 @@ SELECT num_nulls(VARIADIC '{}'::int[]);
2929
-- should fail, one or more arguments is required
3030
SELECT num_nonnulls();
3131
SELECT num_nulls();
32+
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;

0 commit comments

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