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

Commit4eb2611

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 parent52afe56 commit4eb2611

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);
@@ -2599,6 +2598,11 @@ decompile_column_index_array(Datum column_index_array, Oid relId,
25992598
* partial indexes, column default expressions, etc. We also support
26002599
* Var-free expressions, for which the OID can be InvalidOid.
26012600
*
2601+
* If the OID is nonzero but not actually valid, don't throw an error,
2602+
* just return NULL. This is a bit questionable, but it's what we've
2603+
* done historically, and it can help avoid unwanted failures when
2604+
* examining catalog entries for just-deleted relations.
2605+
*
26022606
* We expect this function to work, or throw a reasonably clean error,
26032607
* for any node tree that can appear in a catalog pg_node_tree column.
26042608
* Query trees, such as those appearing in pg_rewrite.ev_action, are
@@ -2611,29 +2615,16 @@ pg_get_expr(PG_FUNCTION_ARGS)
26112615
{
26122616
text*expr=PG_GETARG_TEXT_PP(0);
26132617
Oidrelid=PG_GETARG_OID(1);
2618+
text*result;
26142619
intprettyFlags;
2615-
char*relname;
26162620

26172621
prettyFlags=PRETTYFLAG_INDENT;
26182622

2619-
if (OidIsValid(relid))
2620-
{
2621-
/* Get the name for the relation */
2622-
relname=get_rel_name(relid);
2623-
2624-
/*
2625-
* If the OID isn't actually valid, don't throw an error, just return
2626-
* NULL. This is a bit questionable, but it's what we've done
2627-
* historically, and it can help avoid unwanted failures when
2628-
* examining catalog entries for just-deleted relations.
2629-
*/
2630-
if (relname==NULL)
2631-
PG_RETURN_NULL();
2632-
}
2623+
result=pg_get_expr_worker(expr,relid,prettyFlags);
2624+
if (result)
2625+
PG_RETURN_TEXT_P(result);
26332626
else
2634-
relname=NULL;
2635-
2636-
PG_RETURN_TEXT_P(pg_get_expr_worker(expr,relid,relname,prettyFlags));
2627+
PG_RETURN_NULL();
26372628
}
26382629

26392630
Datum
@@ -2642,33 +2633,27 @@ pg_get_expr_ext(PG_FUNCTION_ARGS)
26422633
text*expr=PG_GETARG_TEXT_PP(0);
26432634
Oidrelid=PG_GETARG_OID(1);
26442635
boolpretty=PG_GETARG_BOOL(2);
2636+
text*result;
26452637
intprettyFlags;
2646-
char*relname;
26472638

26482639
prettyFlags=GET_PRETTY_FLAGS(pretty);
26492640

2650-
if (OidIsValid(relid))
2651-
{
2652-
/* Get the name for the relation */
2653-
relname=get_rel_name(relid);
2654-
/* See notes above */
2655-
if (relname==NULL)
2656-
PG_RETURN_NULL();
2657-
}
2641+
result=pg_get_expr_worker(expr,relid,prettyFlags);
2642+
if (result)
2643+
PG_RETURN_TEXT_P(result);
26582644
else
2659-
relname=NULL;
2660-
2661-
PG_RETURN_TEXT_P(pg_get_expr_worker(expr,relid,relname,prettyFlags));
2645+
PG_RETURN_NULL();
26622646
}
26632647

26642648
statictext*
2665-
pg_get_expr_worker(text*expr,Oidrelid,constchar*relname,intprettyFlags)
2649+
pg_get_expr_worker(text*expr,Oidrelid,intprettyFlags)
26662650
{
26672651
Node*node;
26682652
Node*tst;
26692653
Relidsrelids;
26702654
List*context;
26712655
char*exprstr;
2656+
Relationrel=NULL;
26722657
char*str;
26732658

26742659
/* Convert input pg_node_tree (really TEXT) object to C string */
@@ -2713,16 +2698,29 @@ pg_get_expr_worker(text *expr, Oid relid, const char *relname, int prettyFlags)
27132698
errmsg("expression contains variables")));
27142699
}
27152700

2716-
/* Prepare deparse context if needed */
2701+
/*
2702+
* Prepare deparse context if needed. If we are deparsing with a relid,
2703+
* we need to transiently open and lock the rel, to make sure it won't go
2704+
* away underneath us. (set_relation_column_names would lock it anyway,
2705+
* so this isn't really introducing any new behavior.)
2706+
*/
27172707
if (OidIsValid(relid))
2718-
context=deparse_context_for(relname,relid);
2708+
{
2709+
rel=try_relation_open(relid,AccessShareLock);
2710+
if (rel==NULL)
2711+
returnNULL;
2712+
context=deparse_context_for(RelationGetRelationName(rel),relid);
2713+
}
27192714
else
27202715
context=NIL;
27212716

27222717
/* Deparse */
27232718
str=deparse_expression_pretty(node,context, false, false,
27242719
prettyFlags,0);
27252720

2721+
if (rel!=NULL)
2722+
relation_close(rel,AccessShareLock);
2723+
27262724
returnstring_to_text(str);
27272725
}
27282726

0 commit comments

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