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

Commitd21690e

Browse files
committed
Remove race condition in pg_get_expr().
Since its introduction, pg_get_expr() has intended to silentlyreturn NULL if called with an invalid relation OID, as can happenwhen scanning the catalogs concurrently with relation drops.However, there is a race condition: we check validity of the OIDat the start, but it could get dropped just afterward, leading tofailures. This is the cause of some intermittent instability we'reseeing in a proposed new test case, and presumably it's a hazard inthe field as well.We can fix this by AccessShareLock-ing the target relation for theduration of pg_get_expr(). Since we don't require any permissionson the target relation, this is semantically a bit undesirable. Butit turns out that the set_relation_column_names() subroutine alreadytakes a transient AccessShareLock on that relation, and has done sincecommit2ffa740 in 2012. Given the lack of complaints about that, itseems like there should be no harm in holding the lock a bit longer.Back-patch to all supported branches.Discussion:https://postgr.es/m/31ddcc01-a71b-4e8c-9948-01d1c47293ca@eisentraut.org
1 parent8ead39e commitd21690e

File tree

1 file changed

+33
-35
lines changed

1 file changed

+33
-35
lines changed

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

Lines changed: 33 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -347,8 +347,7 @@ static char *pg_get_partkeydef_worker(Oid relid, int prettyFlags,
347347
boolattrsOnly,boolmissing_ok);
348348
staticchar*pg_get_constraintdef_worker(OidconstraintId,boolfullCommand,
349349
intprettyFlags,boolmissing_ok);
350-
statictext*pg_get_expr_worker(text*expr,Oidrelid,constchar*relname,
351-
intprettyFlags);
350+
statictext*pg_get_expr_worker(text*expr,Oidrelid,intprettyFlags);
352351
staticintprint_function_arguments(StringInfobuf,HeapTupleproctup,
353352
boolprint_table_args,boolprint_defaults);
354353
staticvoidprint_function_rettype(StringInfobuf,HeapTupleproctup);
@@ -2575,36 +2574,28 @@ decompile_column_index_array(Datum column_index_array, Oid relId,
25752574
* the one specified by the second parameter. This is sufficient for
25762575
* partial indexes, column default expressions, etc. We also support
25772576
* Var-free expressions, for which the OID can be InvalidOid.
2577+
*
2578+
* If the OID is nonzero but not actually valid, don't throw an error,
2579+
* just return NULL. This is a bit questionable, but it's what we've
2580+
* done historically, and it can help avoid unwanted failures when
2581+
* examining catalog entries for just-deleted relations.
25782582
* ----------
25792583
*/
25802584
Datum
25812585
pg_get_expr(PG_FUNCTION_ARGS)
25822586
{
25832587
text*expr=PG_GETARG_TEXT_PP(0);
25842588
Oidrelid=PG_GETARG_OID(1);
2589+
text*result;
25852590
intprettyFlags;
2586-
char*relname;
25872591

25882592
prettyFlags=PRETTYFLAG_INDENT;
25892593

2590-
if (OidIsValid(relid))
2591-
{
2592-
/* Get the name for the relation */
2593-
relname=get_rel_name(relid);
2594-
2595-
/*
2596-
* If the OID isn't actually valid, don't throw an error, just return
2597-
* NULL. This is a bit questionable, but it's what we've done
2598-
* historically, and it can help avoid unwanted failures when
2599-
* examining catalog entries for just-deleted relations.
2600-
*/
2601-
if (relname==NULL)
2602-
PG_RETURN_NULL();
2603-
}
2594+
result=pg_get_expr_worker(expr,relid,prettyFlags);
2595+
if (result)
2596+
PG_RETURN_TEXT_P(result);
26042597
else
2605-
relname=NULL;
2606-
2607-
PG_RETURN_TEXT_P(pg_get_expr_worker(expr,relid,relname,prettyFlags));
2598+
PG_RETURN_NULL();
26082599
}
26092600

26102601
Datum
@@ -2613,31 +2604,25 @@ pg_get_expr_ext(PG_FUNCTION_ARGS)
26132604
text*expr=PG_GETARG_TEXT_PP(0);
26142605
Oidrelid=PG_GETARG_OID(1);
26152606
boolpretty=PG_GETARG_BOOL(2);
2607+
text*result;
26162608
intprettyFlags;
2617-
char*relname;
26182609

26192610
prettyFlags=pretty ? (PRETTYFLAG_PAREN |PRETTYFLAG_INDENT |PRETTYFLAG_SCHEMA) :PRETTYFLAG_INDENT;
26202611

2621-
if (OidIsValid(relid))
2622-
{
2623-
/* Get the name for the relation */
2624-
relname=get_rel_name(relid);
2625-
/* See notes above */
2626-
if (relname==NULL)
2627-
PG_RETURN_NULL();
2628-
}
2612+
result=pg_get_expr_worker(expr,relid,prettyFlags);
2613+
if (result)
2614+
PG_RETURN_TEXT_P(result);
26292615
else
2630-
relname=NULL;
2631-
2632-
PG_RETURN_TEXT_P(pg_get_expr_worker(expr,relid,relname,prettyFlags));
2616+
PG_RETURN_NULL();
26332617
}
26342618

26352619
statictext*
2636-
pg_get_expr_worker(text*expr,Oidrelid,constchar*relname,intprettyFlags)
2620+
pg_get_expr_worker(text*expr,Oidrelid,intprettyFlags)
26372621
{
26382622
Node*node;
26392623
List*context;
26402624
char*exprstr;
2625+
Relationrel=NULL;
26412626
char*str;
26422627

26432628
/* Convert input TEXT object to C string */
@@ -2648,16 +2633,29 @@ pg_get_expr_worker(text *expr, Oid relid, const char *relname, int prettyFlags)
26482633

26492634
pfree(exprstr);
26502635

2651-
/* Prepare deparse context if needed */
2636+
/*
2637+
* Prepare deparse context if needed. If we are deparsing with a relid,
2638+
* we need to transiently open and lock the rel, to make sure it won't go
2639+
* away underneath us. (set_relation_column_names would lock it anyway,
2640+
* so this isn't really introducing any new behavior.)
2641+
*/
26522642
if (OidIsValid(relid))
2653-
context=deparse_context_for(relname,relid);
2643+
{
2644+
rel=try_relation_open(relid,AccessShareLock);
2645+
if (rel==NULL)
2646+
returnNULL;
2647+
context=deparse_context_for(RelationGetRelationName(rel),relid);
2648+
}
26542649
else
26552650
context=NIL;
26562651

26572652
/* Deparse */
26582653
str=deparse_expression_pretty(node,context, false, false,
26592654
prettyFlags,0);
26602655

2656+
if (rel!=NULL)
2657+
relation_close(rel,AccessShareLock);
2658+
26612659
returnstring_to_text(str);
26622660
}
26632661

0 commit comments

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