Re: SQL:2011 application time - Mailing list pgsql-hackers

From jian he
Subject Re: SQL:2011 application time
Date
Msg-id CACJufxGsjGXbKT-qbPYtspLxN74B8Q8dBXRZV+HfgqC0hh-ZNQ@mail.gmail.com
Whole thread Raw
In response to Re: SQL:2011 application time  (jian he <jian.universality@gmail.com>)
Responses Re: SQL:2011 application time
List pgsql-hackers
Hi.
based on v16.

/* Look up the FOR PORTION OF name requested. */
range_attno = attnameAttNum(targetrel, range_name, false);
if (range_attno == InvalidAttrNumber)
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_COLUMN),
errmsg("column or period \"%s\" of relation \"%s\" does not exist",
range_name,
RelationGetRelationName(targetrel)),
parser_errposition(pstate, forPortionOf->range_name_location)));
attr = TupleDescAttr(targetrel->rd_att, range_attno - 1);
// TODO: check attr->attisdropped (?),
// and figure out concurrency issues with that in general.
// It should work the same as updating any other column.

I don't think we need to check attr->attisdropped here.
because the above function attnameAttNum already does the job.
--------------------------------------------
bool
get_typname_and_namespace(Oid typid, char **typname, char **typnamespace)
{
HeapTuple tp;

tp = SearchSysCache1(TYPEOID, ObjectIdGetDatum(typid));
if (HeapTupleIsValid(tp))
{
Form_pg_type typtup = (Form_pg_type) GETSTRUCT(tp);

*typname = pstrdup(NameStr(typtup->typname));
*typnamespace = get_namespace_name(typtup->typnamespace);
ReleaseSysCache(tp);
return *typnamespace;

"return *typnamespace;" should be "return true"?
Maybe name it  to get_typname_and_typnamespace?
-----------------------------------------------------------------------
if (!get_typname_and_namespace(attr->atttypid, &range_type_name,
&range_type_namespace))
elog(ERROR, "missing range type %d", attr->atttypid);

you can just `elog(ERROR, "missing range type %s", range_type_name);` ?
Also, this should be placed just below if (!type_is_range(attr->atttypid))?
-----------------------------------------------------------------------
src/backend/catalog/objectaddress.c

if (OidIsValid(per->perrelid))
{
StringInfoData rel;

initStringInfo(&rel);
getRelationDescription(&rel, per->perrelid, false);
appendStringInfo(&buffer, _("period %s on %s"),
NameStr(per->pername), rel.data);
pfree(rel.data);
}
else
{
appendStringInfo(&buffer, _("period %s"),
NameStr(per->pername));
}

periods are always associated with the table, is the above else branch correct?
-----------------------------------------------------------------------
File: src/backend/commands/tablecmds.c
7899: /*
7900: * this test is deliberately not attisdropped-aware, since if one tries to
7901: * add a column matching a dropped column name, it's gonna fail anyway.
7902: *
7903: * XXX: Does this hold for periods?
7904: */
7905: attTuple = SearchSysCache2(ATTNAME,
7906:    ObjectIdGetDatum(RelationGetRelid(rel)),
7907:    PointerGetDatum(pername));

XXX: Does this hold for periods?
Yes. we can add the following 2 sql for code coverage.
alter table pt add period for tableoid (ds, de);
alter table pt add period for "........pg.dropped.4........" (ds, de);



pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: Add the ability to limit the amount of memory that can be allocated to backends.
Next
From: Jakub Wartak
Date:
Subject: Re: trying again to get incremental backup