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

Commitce57143

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 parenta584d03 commitce57143

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);
@@ -2615,6 +2614,11 @@ decompile_column_index_array(Datum column_index_array, Oid relId,
26152614
* partial indexes, column default expressions, etc. We also support
26162615
* Var-free expressions, for which the OID can be InvalidOid.
26172616
*
2617+
* If the OID is nonzero but not actually valid, don't throw an error,
2618+
* just return NULL. This is a bit questionable, but it's what we've
2619+
* done historically, and it can help avoid unwanted failures when
2620+
* examining catalog entries for just-deleted relations.
2621+
*
26182622
* We expect this function to work, or throw a reasonably clean error,
26192623
* for any node tree that can appear in a catalog pg_node_tree column.
26202624
* Query trees, such as those appearing in pg_rewrite.ev_action, are
@@ -2627,29 +2631,16 @@ pg_get_expr(PG_FUNCTION_ARGS)
26272631
{
26282632
text*expr=PG_GETARG_TEXT_PP(0);
26292633
Oidrelid=PG_GETARG_OID(1);
2634+
text*result;
26302635
intprettyFlags;
2631-
char*relname;
26322636

26332637
prettyFlags=PRETTYFLAG_INDENT;
26342638

2635-
if (OidIsValid(relid))
2636-
{
2637-
/* Get the name for the relation */
2638-
relname=get_rel_name(relid);
2639-
2640-
/*
2641-
* If the OID isn't actually valid, don't throw an error, just return
2642-
* NULL. This is a bit questionable, but it's what we've done
2643-
* historically, and it can help avoid unwanted failures when
2644-
* examining catalog entries for just-deleted relations.
2645-
*/
2646-
if (relname==NULL)
2647-
PG_RETURN_NULL();
2648-
}
2639+
result=pg_get_expr_worker(expr,relid,prettyFlags);
2640+
if (result)
2641+
PG_RETURN_TEXT_P(result);
26492642
else
2650-
relname=NULL;
2651-
2652-
PG_RETURN_TEXT_P(pg_get_expr_worker(expr,relid,relname,prettyFlags));
2643+
PG_RETURN_NULL();
26532644
}
26542645

26552646
Datum
@@ -2658,33 +2649,27 @@ pg_get_expr_ext(PG_FUNCTION_ARGS)
26582649
text*expr=PG_GETARG_TEXT_PP(0);
26592650
Oidrelid=PG_GETARG_OID(1);
26602651
boolpretty=PG_GETARG_BOOL(2);
2652+
text*result;
26612653
intprettyFlags;
2662-
char*relname;
26632654

26642655
prettyFlags=GET_PRETTY_FLAGS(pretty);
26652656

2666-
if (OidIsValid(relid))
2667-
{
2668-
/* Get the name for the relation */
2669-
relname=get_rel_name(relid);
2670-
/* See notes above */
2671-
if (relname==NULL)
2672-
PG_RETURN_NULL();
2673-
}
2657+
result=pg_get_expr_worker(expr,relid,prettyFlags);
2658+
if (result)
2659+
PG_RETURN_TEXT_P(result);
26742660
else
2675-
relname=NULL;
2676-
2677-
PG_RETURN_TEXT_P(pg_get_expr_worker(expr,relid,relname,prettyFlags));
2661+
PG_RETURN_NULL();
26782662
}
26792663

26802664
statictext*
2681-
pg_get_expr_worker(text*expr,Oidrelid,constchar*relname,intprettyFlags)
2665+
pg_get_expr_worker(text*expr,Oidrelid,intprettyFlags)
26822666
{
26832667
Node*node;
26842668
Node*tst;
26852669
Relidsrelids;
26862670
List*context;
26872671
char*exprstr;
2672+
Relationrel=NULL;
26882673
char*str;
26892674

26902675
/* Convert input pg_node_tree (really TEXT) object to C string */
@@ -2729,16 +2714,29 @@ pg_get_expr_worker(text *expr, Oid relid, const char *relname, int prettyFlags)
27292714
errmsg("expression contains variables")));
27302715
}
27312716

2732-
/* Prepare deparse context if needed */
2717+
/*
2718+
* Prepare deparse context if needed. If we are deparsing with a relid,
2719+
* we need to transiently open and lock the rel, to make sure it won't go
2720+
* away underneath us. (set_relation_column_names would lock it anyway,
2721+
* so this isn't really introducing any new behavior.)
2722+
*/
27332723
if (OidIsValid(relid))
2734-
context=deparse_context_for(relname,relid);
2724+
{
2725+
rel=try_relation_open(relid,AccessShareLock);
2726+
if (rel==NULL)
2727+
returnNULL;
2728+
context=deparse_context_for(RelationGetRelationName(rel),relid);
2729+
}
27352730
else
27362731
context=NIL;
27372732

27382733
/* Deparse */
27392734
str=deparse_expression_pretty(node,context, false, false,
27402735
prettyFlags,0);
27412736

2737+
if (rel!=NULL)
2738+
relation_close(rel,AccessShareLock);
2739+
27422740
returnstring_to_text(str);
27432741
}
27442742

0 commit comments

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