On Wed, Oct 22, 2025 at 12:51 PM jian he <[email protected]> wrote:
> On Thu, Oct 16, 2025 at 8:51 PM Akshay Joshi > <[email protected]> wrote: > > > > Please find attached the v3 patch, which resolves all compilation errors > and warnings. > > > > drop table if exists t, ts, ts1; > create table t(a int); > CREATE POLICY p0 ON t FOR ALL TO PUBLIC USING (a % 2 = 1); > SELECT pg_get_policy_ddl('t', 'p0', false); > > pg_get_policy_ddl > --------------------------------------------------------------------- > CREATE POLICY p0 ON t AS PERMISSIVE FOR ALL USING (((a % 2) = 1)); > (1 row) > > "TO PUBLIC" part is missing, maybe it's ok. > I used the logic below, which did not return PUBLIC as a role. I have added logic to default the TO clause to PUBLIC when no specific role name is provided valueDatum = heap_getattr(tuplePolicy, Anum_pg_policy_polroles, RelationGetDescr(pgPolicyRel), &attrIsNull); if (!attrIsNull) { ArrayType *policy_roles = DatumGetArrayTypePCopy(valueDatum); int nitems = ARR_DIMS(policy_roles)[0]; Oid *roles = (Oid *) ARR_DATA_PTR(policy_roles); > > > SELECT pg_get_policy_ddl(-1, 'p0', false); > ERROR: could not open relation with OID 4294967295 > as I mentioned in a nearby thread [1], this should be NULL instead of > ERROR. > [1] > https://postgr.es/m/CACJufxGbE4uJWu1YuqdmOx+7PMBpHvX_fbRMmHu=r4srsuw...@mail.gmail.com > > Fixed in v4 patch. > > IMHO, get_formatted_string is not needed, most of the time, if pretty is > true, > we append "\t" and "\n", for that we can simply do > ``` > appendStringInfo(&buf, "CREATE POLICY %s ON %s ", > quote_identifier(NameStr(*policyName)), > generate_qualified_relation_name(policy_form->polrelid)); > if (pretty) > appendStringInfoString(buf, "\t\n"); > ``` > > The get_formatted_string function is needed. Instead of using multiple if statements for the pretty flag, it’s better to have a generic function. This will be useful if the pretty-format DDL implementation is accepted by the community, as it can be reused by other pg_get_<object>_ddl() DDL functions added in the future. > > in pg_get_triggerdef_worker, I found the below code pattern: > /* > * In non-pretty mode, always schema-qualify the target table name for > * safety. In pretty mode, schema-qualify only if not visible. > */ > appendStringInfo(&buf, " ON %s ", > pretty ? > generate_relation_name(trigrec->tgrelid, NIL) : > generate_qualified_relation_name(trigrec->tgrelid)); > > maybe we can apply it too while construct query string: > "CREATE POLICY %s ON %s", > In my opinion, the table name should always be schema-qualified, which I have addressed in the v4 patch. The implementation of the pretty flag here differs from pg_get_triggerdef_worker, which is used only for the target table name. In my patch, the pretty flag adds \t and \n to each different clause (example AS, FOR, USING ...)
v4-0001-Add-pg_get_policy_ddl-function-to-reconstruct-CREATE.patch
Description: Binary data
