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

Commitceb224b

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 parent8c5da20 commitceb224b

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
@@ -344,8 +344,7 @@ static char *pg_get_partkeydef_worker(Oid relid, int prettyFlags,
344344
boolattrsOnly,boolmissing_ok);
345345
staticchar*pg_get_constraintdef_worker(OidconstraintId,boolfullCommand,
346346
intprettyFlags,boolmissing_ok);
347-
statictext*pg_get_expr_worker(text*expr,Oidrelid,constchar*relname,
348-
intprettyFlags);
347+
statictext*pg_get_expr_worker(text*expr,Oidrelid,intprettyFlags);
349348
staticintprint_function_arguments(StringInfobuf,HeapTupleproctup,
350349
boolprint_table_args,boolprint_defaults);
351350
staticvoidprint_function_rettype(StringInfobuf,HeapTupleproctup);
@@ -2384,36 +2383,28 @@ decompile_column_index_array(Datum column_index_array, Oid relId,
23842383
* the one specified by the second parameter. This is sufficient for
23852384
* partial indexes, column default expressions, etc. We also support
23862385
* Var-free expressions, for which the OID can be InvalidOid.
2386+
*
2387+
* If the OID is nonzero but not actually valid, don't throw an error,
2388+
* just return NULL. This is a bit questionable, but it's what we've
2389+
* done historically, and it can help avoid unwanted failures when
2390+
* examining catalog entries for just-deleted relations.
23872391
* ----------
23882392
*/
23892393
Datum
23902394
pg_get_expr(PG_FUNCTION_ARGS)
23912395
{
23922396
text*expr=PG_GETARG_TEXT_PP(0);
23932397
Oidrelid=PG_GETARG_OID(1);
2398+
text*result;
23942399
intprettyFlags;
2395-
char*relname;
23962400

23972401
prettyFlags=PRETTYFLAG_INDENT;
23982402

2399-
if (OidIsValid(relid))
2400-
{
2401-
/* Get the name for the relation */
2402-
relname=get_rel_name(relid);
2403-
2404-
/*
2405-
* If the OID isn't actually valid, don't throw an error, just return
2406-
* NULL. This is a bit questionable, but it's what we've done
2407-
* historically, and it can help avoid unwanted failures when
2408-
* examining catalog entries for just-deleted relations.
2409-
*/
2410-
if (relname==NULL)
2411-
PG_RETURN_NULL();
2412-
}
2403+
result=pg_get_expr_worker(expr,relid,prettyFlags);
2404+
if (result)
2405+
PG_RETURN_TEXT_P(result);
24132406
else
2414-
relname=NULL;
2415-
2416-
PG_RETURN_TEXT_P(pg_get_expr_worker(expr,relid,relname,prettyFlags));
2407+
PG_RETURN_NULL();
24172408
}
24182409

24192410
Datum
@@ -2422,31 +2413,25 @@ pg_get_expr_ext(PG_FUNCTION_ARGS)
24222413
text*expr=PG_GETARG_TEXT_PP(0);
24232414
Oidrelid=PG_GETARG_OID(1);
24242415
boolpretty=PG_GETARG_BOOL(2);
2416+
text*result;
24252417
intprettyFlags;
2426-
char*relname;
24272418

24282419
prettyFlags=pretty ? (PRETTYFLAG_PAREN |PRETTYFLAG_INDENT |PRETTYFLAG_SCHEMA) :PRETTYFLAG_INDENT;
24292420

2430-
if (OidIsValid(relid))
2431-
{
2432-
/* Get the name for the relation */
2433-
relname=get_rel_name(relid);
2434-
/* See notes above */
2435-
if (relname==NULL)
2436-
PG_RETURN_NULL();
2437-
}
2421+
result=pg_get_expr_worker(expr,relid,prettyFlags);
2422+
if (result)
2423+
PG_RETURN_TEXT_P(result);
24382424
else
2439-
relname=NULL;
2440-
2441-
PG_RETURN_TEXT_P(pg_get_expr_worker(expr,relid,relname,prettyFlags));
2425+
PG_RETURN_NULL();
24422426
}
24432427

24442428
statictext*
2445-
pg_get_expr_worker(text*expr,Oidrelid,constchar*relname,intprettyFlags)
2429+
pg_get_expr_worker(text*expr,Oidrelid,intprettyFlags)
24462430
{
24472431
Node*node;
24482432
List*context;
24492433
char*exprstr;
2434+
Relationrel=NULL;
24502435
char*str;
24512436

24522437
/* Convert input TEXT object to C string */
@@ -2457,16 +2442,29 @@ pg_get_expr_worker(text *expr, Oid relid, const char *relname, int prettyFlags)
24572442

24582443
pfree(exprstr);
24592444

2460-
/* Prepare deparse context if needed */
2445+
/*
2446+
* Prepare deparse context if needed. If we are deparsing with a relid,
2447+
* we need to transiently open and lock the rel, to make sure it won't go
2448+
* away underneath us. (set_relation_column_names would lock it anyway,
2449+
* so this isn't really introducing any new behavior.)
2450+
*/
24612451
if (OidIsValid(relid))
2462-
context=deparse_context_for(relname,relid);
2452+
{
2453+
rel=try_relation_open(relid,AccessShareLock);
2454+
if (rel==NULL)
2455+
returnNULL;
2456+
context=deparse_context_for(RelationGetRelationName(rel),relid);
2457+
}
24632458
else
24642459
context=NIL;
24652460

24662461
/* Deparse */
24672462
str=deparse_expression_pretty(node,context, false, false,
24682463
prettyFlags,0);
24692464

2465+
if (rel!=NULL)
2466+
relation_close(rel,AccessShareLock);
2467+
24702468
returnstring_to_text(str);
24712469
}
24722470

0 commit comments

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