Re: SQL:2011 application time

From: jian he <jian(dot)universality(at)gmail(dot)com>
To: Paul Jungwirth <pj(at)illuminatedcomputing(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: SQL:2011 application time
Date: 2023-10-20 12:45:05
Message-ID: CACJufxGsjGXbKT-qbPYtspLxN74B8Q8dBXRZV+HfgqC0hh-ZNQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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);

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jakub Wartak 2023-10-20 13:20:10 Re: trying again to get incremental backup
Previous Message Stephen Frost 2023-10-20 12:39:33 Re: Add the ability to limit the amount of memory that can be allocated to backends.