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

Commit26c89d1

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 parent806f989 commit26c89d1

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
@@ -352,8 +352,7 @@ static char *pg_get_partkeydef_worker(Oid relid, int prettyFlags,
352352
boolattrsOnly,boolmissing_ok);
353353
staticchar*pg_get_constraintdef_worker(OidconstraintId,boolfullCommand,
354354
intprettyFlags,boolmissing_ok);
355-
statictext*pg_get_expr_worker(text*expr,Oidrelid,constchar*relname,
356-
intprettyFlags);
355+
statictext*pg_get_expr_worker(text*expr,Oidrelid,intprettyFlags);
357356
staticintprint_function_arguments(StringInfobuf,HeapTupleproctup,
358357
boolprint_table_args,boolprint_defaults);
359358
staticvoidprint_function_rettype(StringInfobuf,HeapTupleproctup);
@@ -2630,6 +2629,11 @@ decompile_column_index_array(Datum column_index_array, Oid relId,
26302629
* partial indexes, column default expressions, etc. We also support
26312630
* Var-free expressions, for which the OID can be InvalidOid.
26322631
*
2632+
* If the OID is nonzero but not actually valid, don't throw an error,
2633+
* just return NULL. This is a bit questionable, but it's what we've
2634+
* done historically, and it can help avoid unwanted failures when
2635+
* examining catalog entries for just-deleted relations.
2636+
*
26332637
* We expect this function to work, or throw a reasonably clean error,
26342638
* for any node tree that can appear in a catalog pg_node_tree column.
26352639
* Query trees, such as those appearing in pg_rewrite.ev_action, are
@@ -2642,29 +2646,16 @@ pg_get_expr(PG_FUNCTION_ARGS)
26422646
{
26432647
text*expr=PG_GETARG_TEXT_PP(0);
26442648
Oidrelid=PG_GETARG_OID(1);
2649+
text*result;
26452650
intprettyFlags;
2646-
char*relname;
26472651

26482652
prettyFlags=PRETTYFLAG_INDENT;
26492653

2650-
if (OidIsValid(relid))
2651-
{
2652-
/* Get the name for the relation */
2653-
relname=get_rel_name(relid);
2654-
2655-
/*
2656-
* If the OID isn't actually valid, don't throw an error, just return
2657-
* NULL. This is a bit questionable, but it's what we've done
2658-
* historically, and it can help avoid unwanted failures when
2659-
* examining catalog entries for just-deleted relations.
2660-
*/
2661-
if (relname==NULL)
2662-
PG_RETURN_NULL();
2663-
}
2654+
result=pg_get_expr_worker(expr,relid,prettyFlags);
2655+
if (result)
2656+
PG_RETURN_TEXT_P(result);
26642657
else
2665-
relname=NULL;
2666-
2667-
PG_RETURN_TEXT_P(pg_get_expr_worker(expr,relid,relname,prettyFlags));
2658+
PG_RETURN_NULL();
26682659
}
26692660

26702661
Datum
@@ -2673,33 +2664,27 @@ pg_get_expr_ext(PG_FUNCTION_ARGS)
26732664
text*expr=PG_GETARG_TEXT_PP(0);
26742665
Oidrelid=PG_GETARG_OID(1);
26752666
boolpretty=PG_GETARG_BOOL(2);
2667+
text*result;
26762668
intprettyFlags;
2677-
char*relname;
26782669

26792670
prettyFlags=GET_PRETTY_FLAGS(pretty);
26802671

2681-
if (OidIsValid(relid))
2682-
{
2683-
/* Get the name for the relation */
2684-
relname=get_rel_name(relid);
2685-
/* See notes above */
2686-
if (relname==NULL)
2687-
PG_RETURN_NULL();
2688-
}
2672+
result=pg_get_expr_worker(expr,relid,prettyFlags);
2673+
if (result)
2674+
PG_RETURN_TEXT_P(result);
26892675
else
2690-
relname=NULL;
2691-
2692-
PG_RETURN_TEXT_P(pg_get_expr_worker(expr,relid,relname,prettyFlags));
2676+
PG_RETURN_NULL();
26932677
}
26942678

26952679
statictext*
2696-
pg_get_expr_worker(text*expr,Oidrelid,constchar*relname,intprettyFlags)
2680+
pg_get_expr_worker(text*expr,Oidrelid,intprettyFlags)
26972681
{
26982682
Node*node;
26992683
Node*tst;
27002684
Relidsrelids;
27012685
List*context;
27022686
char*exprstr;
2687+
Relationrel=NULL;
27032688
char*str;
27042689

27052690
/* Convert input pg_node_tree (really TEXT) object to C string */
@@ -2744,16 +2729,29 @@ pg_get_expr_worker(text *expr, Oid relid, const char *relname, int prettyFlags)
27442729
errmsg("expression contains variables")));
27452730
}
27462731

2747-
/* Prepare deparse context if needed */
2732+
/*
2733+
* Prepare deparse context if needed. If we are deparsing with a relid,
2734+
* we need to transiently open and lock the rel, to make sure it won't go
2735+
* away underneath us. (set_relation_column_names would lock it anyway,
2736+
* so this isn't really introducing any new behavior.)
2737+
*/
27482738
if (OidIsValid(relid))
2749-
context=deparse_context_for(relname,relid);
2739+
{
2740+
rel=try_relation_open(relid,AccessShareLock);
2741+
if (rel==NULL)
2742+
returnNULL;
2743+
context=deparse_context_for(RelationGetRelationName(rel),relid);
2744+
}
27502745
else
27512746
context=NIL;
27522747

27532748
/* Deparse */
27542749
str=deparse_expression_pretty(node,context, false, false,
27552750
prettyFlags,0);
27562751

2752+
if (rel!=NULL)
2753+
relation_close(rel,AccessShareLock);
2754+
27572755
returnstring_to_text(str);
27582756
}
27592757

0 commit comments

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