Re: Fast default stuff versus pg_upgrade
От | Andrew Dunstan |
---|---|
Тема | Re: Fast default stuff versus pg_upgrade |
Дата | |
Msg-id | b0f049e0-a6e6-a5f1-563c-171d2e0999f5@2ndQuadrant.com обсуждение исходный текст |
Ответ на | Re: Fast default stuff versus pg_upgrade (Andres Freund <andres@anarazel.de>) |
Ответы |
Re: Fast default stuff versus pg_upgrade
(Andrew Dunstan <andrew.dunstan@2ndquadrant.com>)
|
Список | pgsql-hackers |
On 06/20/2018 12:58 PM, Andres Freund wrote: > Hi, > > Just a quick scan-through review: > > On 2018-06-20 12:51:07 -0400, Andrew Dunstan wrote: >> diff --git a/src/backend/utils/adt/pg_upgrade_support.c b/src/backend/utils/adt/pg_upgrade_support.c >> index 0c54b02..2666ab2 100644 >> --- a/src/backend/utils/adt/pg_upgrade_support.c >> +++ b/src/backend/utils/adt/pg_upgrade_support.c >> @@ -11,14 +11,19 @@ >> >> #include "postgres.h" >> >> +#include "access/heapam.h" >> +#include "access/htup_details.h" >> #include "catalog/binary_upgrade.h" >> +#include "catalog/indexing.h" >> #include "catalog/namespace.h" >> #include "catalog/pg_type.h" >> #include "commands/extension.h" >> #include "miscadmin.h" >> #include "utils/array.h" >> #include "utils/builtins.h" >> - >> +#include "utils/lsyscache.h" >> +#include "utils/rel.h" >> +#include "utils/syscache.h" >> > Seems to delete a newline. Will fix > > >> #define CHECK_IS_BINARY_UPGRADE \ >> do { \ >> @@ -192,3 +197,66 @@ binary_upgrade_set_record_init_privs(PG_FUNCTION_ARGS) >> >> PG_RETURN_VOID(); >> } >> + >> +Datum >> +binary_upgrade_set_missing_value(PG_FUNCTION_ARGS) >> +{ >> + Oid table_id = PG_GETARG_OID(0); >> + text *attname = PG_GETARG_TEXT_P(1); >> + text *value = PG_GETARG_TEXT_P(2); >> + Datum valuesAtt[Natts_pg_attribute]; >> + bool nullsAtt[Natts_pg_attribute]; >> + bool replacesAtt[Natts_pg_attribute]; >> + Datum missingval; >> + Form_pg_attribute attStruct; >> + Relation attrrel; >> + HeapTuple atttup, newtup; >> + Oid inputfunc, inputparam; >> + char *cattname = text_to_cstring(attname); >> + char *cvalue = text_to_cstring(value); >> + >> + CHECK_IS_BINARY_UPGRADE; >> + >> + /* Lock the attribute row and get the data */ >> + attrrel = heap_open(AttributeRelationId, RowExclusiveLock); > Maybe I'm being paranoid, but I feel like the relation should also be > locked here. I wondered about that. Other opinions? > > >> + atttup = SearchSysCacheAttName(table_id,cattname); > Missing space before 'cattname'. Will fix. > >> + if (!HeapTupleIsValid(atttup)) >> + elog(ERROR, "cache lookup failed for attribute %s of relation %u", >> + cattname, table_id); >> + attStruct = (Form_pg_attribute) GETSTRUCT(atttup); >> + >> + /* get an array value from the value string */ >> + >> + /* find the io info for an arbitrary array type to get array_in Oid */ >> + getTypeInputInfo(INT4ARRAYOID, &inputfunc, &inputparam); > Maybe I'm confused, but why INT4ARRAYOID? If you're just looking for the > oid of array_in, why not use F_ARRAY_IN? Brain fade? Will fix. > >> + missingval = OidFunctionCall3( >> + inputfunc, /* array_in */ >> + CStringGetDatum(cvalue), >> + ObjectIdGetDatum(attStruct->atttypid), >> + Int32GetDatum(attStruct->atttypmod)); >> + >> + /* update the tuple - set atthasmissing and attmissingval */ >> + MemSet(valuesAtt, 0, sizeof(valuesAtt)); >> + MemSet(nullsAtt, false, sizeof(nullsAtt)); >> + MemSet(replacesAtt, false, sizeof(replacesAtt)); >> + >> + valuesAtt[Anum_pg_attribute_atthasmissing - 1] = BoolGetDatum(true); >> + replacesAtt[Anum_pg_attribute_atthasmissing - 1] = true; >> + valuesAtt[Anum_pg_attribute_attmissingval - 1] = missingval; >> + replacesAtt[Anum_pg_attribute_attmissingval - 1] = true; >> + >> + newtup = heap_modify_tuple(atttup, RelationGetDescr(attrrel), >> + valuesAtt, nullsAtt, replacesAtt); >> + CatalogTupleUpdate(attrrel, &newtup->t_self, newtup); >> + /* clean up */ >> + heap_freetuple(newtup); >> + ReleaseSysCache(atttup); >> + pfree(cattname); >> + pfree(cvalue); >> + pfree(DatumGetPointer(missingval)); > Is this worth bothering with (except the ReleaseSysCache)? We'll clean > up via memory context soon, no? > Done from an abundance of caution. I'll remove them. Thanks for the quick review. Attached deals with all those issues except locking. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
В списке pgsql-hackers по дате отправления:
Следующее
От: Andres FreundДата:
Сообщение: Re: Avoiding Tablespace path collision for primary and standby