public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 4/8] Optimize allocations in bringetbitmap
3+ messages / 3 participants
[nested] [flat]
* [PATCH 4/8] Optimize allocations in bringetbitmap
@ 2021-03-02 18:57 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Tomas Vondra @ 2021-03-02 18:57 UTC (permalink / raw)
The bringetbitmap function allocates memory for various purposes, which
may be quite expensive, depending on the number of scan keys. Instead of
allocating them separately, allocate one bit chunk of memory an carve it
into smaller pieces as needed - all the pieces have the same lifespan,
and it saves quite a bit of CPU and memory overhead.
Author: Tomas Vondra <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/backend/access/brin/brin.c | 60 ++++++++++++++++++++++++++--------
1 file changed, 47 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index ce0f525c21..758c47f0f4 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -373,6 +373,9 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
int *nkeys,
*nnullkeys;
int keyno;
+ char *ptr;
+ Size len;
+ char *tmp PG_USED_FOR_ASSERTS_ONLY;
opaque = (BrinOpaque *) scan->opaque;
bdesc = opaque->bo_bdesc;
@@ -402,15 +405,52 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
* We keep null and regular keys separate, so that we can pass just the
* regular keys to the consistent function easily.
*
+ * To reduce the allocation overhead, we allocate one big chunk and then
+ * carve it into smaller arrays ourselves. All the pieces have exactly the
+ * same lifetime, so that's OK.
+ *
* XXX The widest table can have ~1600 attributes, so this may allocate a
* couple kilobytes of memory). We could invent a more compact approach
* (with just space for used attributes) but that would make the matching
* more complicated, so it may not be a win.
*/
- keys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
- nullkeys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
- nkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts);
- nnullkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts);
+ len =
+ MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts) + /* regular keys */
+ MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys) * bdesc->bd_tupdesc->natts +
+ MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts) +
+ MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts) + /* NULL keys */
+ MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys) * bdesc->bd_tupdesc->natts +
+ MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts);
+
+ ptr = palloc(len);
+ tmp = ptr;
+
+ keys = (ScanKey **) ptr;
+ ptr += MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
+
+ nullkeys = (ScanKey **) ptr;
+ ptr += MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
+
+ nkeys = (int *) ptr;
+ ptr += MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts);
+
+ nnullkeys = (int *) ptr;
+ ptr += MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts);
+
+ for (int i = 0; i < bdesc->bd_tupdesc->natts; i++)
+ {
+ keys[i] = (ScanKey *) ptr;
+ ptr += MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys);
+
+ nullkeys[i] = (ScanKey *) ptr;
+ ptr += MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys);
+ }
+
+ Assert(tmp + len == ptr);
+
+ /* zero the number of keys */
+ memset(nkeys, 0, sizeof(int) * bdesc->bd_tupdesc->natts);
+ memset(nnullkeys, 0, sizeof(int) * bdesc->bd_tupdesc->natts);
/*
* Preprocess the scan keys - split them into per-attribute arrays.
@@ -444,9 +484,9 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
{
FmgrInfo *tmp;
- /* No key/null arrays for this attribute. */
- Assert((keys[keyattno - 1] == NULL) && (nkeys[keyattno - 1] == 0));
- Assert((nullkeys[keyattno - 1] == NULL) && (nnullkeys[keyattno - 1] == 0));
+ /* First time we see this attribute, so no key/null keys. */
+ Assert(nkeys[keyattno - 1] == 0);
+ Assert(nnullkeys[keyattno - 1] == 0);
tmp = index_getprocinfo(idxRel, keyattno,
BRIN_PROCNUM_CONSISTENT);
@@ -457,17 +497,11 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
/* Add key to the proper per-attribute array. */
if (key->sk_flags & SK_ISNULL)
{
- if (!nullkeys[keyattno - 1])
- nullkeys[keyattno - 1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys);
-
nullkeys[keyattno - 1][nnullkeys[keyattno - 1]] = key;
nnullkeys[keyattno - 1]++;
}
else
{
- if (!keys[keyattno - 1])
- keys[keyattno - 1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys);
-
keys[keyattno - 1][nkeys[keyattno - 1]] = key;
nkeys[keyattno - 1]++;
}
--
2.26.2
--------------D524CD37E77B36A2883A7617
Content-Type: text/x-patch; charset=UTF-8;
name="0005-BRIN-bloom-indexes-20210311.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment;
filename="0005-BRIN-bloom-indexes-20210311.patch"
^ permalink raw reply [nested|flat] 3+ messages in thread
* Add pg_get_acl() function get the ACL for a database object
@ 2024-06-19 11:34 Joel Jacobson <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Joel Jacobson @ 2024-06-19 11:34 UTC (permalink / raw)
To: pgsql-hackers
Hello hackers,
Currently, obtaining the Access Control List (ACL) for a database object
requires querying specific pg_catalog tables directly, where the user
needs to know the name of the ACL column for the object.
Consider:
```
CREATE USER test_user;
CREATE USER test_owner;
CREATE SCHEMA test_schema AUTHORIZATION test_owner;
SET ROLE TO test_owner;
CREATE TABLE test_schema.test_table ();
GRANT SELECT ON TABLE test_schema.test_table TO test_user;
```
To get the ACL we can do:
```
SELECT relacl FROM pg_class WHERE oid = 'test_schema.test_table'::regclass::oid;
relacl
---------------------------------------------------------
{test_owner=arwdDxtm/test_owner,test_user=r/test_owner}
```
Attached patch adds a new SQL-callable functoin `pg_get_acl()`, so we can do:
```
SELECT pg_get_acl('pg_class'::regclass, 'test_schema.test_table'::regclass::oid);
pg_get_acl
---------------------------------------------------------
{test_owner=arwdDxtm/test_owner,test_user=r/test_owner}
```
The original idea for this function came from Alvaro Herrera,
in this related discussion:
https://postgr.es/m/[email protected]
On Thu, Mar 25, 2021, at 16:16, Alvaro Herrera wrote:
> On 2021-Mar-25, Joel Jacobson wrote:
>
>> pg_shdepend doesn't contain the aclitem info though,
>> so it won't work for pg_permissions if we want to expose
>> privilege_type, is_grantable and grantor.
>
> Ah, of course -- the only way to obtain the acl columns is by going
> through the catalogs individually, so it won't be possible. I think
> this could be fixed with some very simple, quick function pg_get_acl()
> that takes a catalog OID and object OID and returns the ACL; then
> use aclexplode() to obtain all those details.
The pg_get_acl() function has been implemented by following
the guidance from Alvaro in the related dicussion:
On Fri, Mar 26, 2021, at 13:43, Alvaro Herrera wrote:
> AFAICS the way to do it is like AlterObjectOwner_internal obtains data
> -- first do get_catalog_object_by_oid (gives you the HeapTuple that
> represents the object), then
> heap_getattr( ..., get_object_attnum_acl(), ..), and there you have the
> ACL which you can "explode" (or maybe just return as-is).
>
> AFAICS if you do this, it's just one cache lookups per object, or
> one indexscan for the cases with no by-OID syscache. It should be much
> cheaper than the UNION ALL query. And you use pg_shdepend to guide
> this, so you only do it for the objects that you already know are
> interesting.
Many thanks Alvaro for the very helpful instructions.
This function would then allow users to e.g. create a view to show the privileges
for all database objects, like the pg_privileges system view suggested in the
related discussion.
Tests and docs are added.
Best regards,
Joel Jakobsson
Attachments:
[application/octet-stream] 0001-Add-pg_get_acl.patch (6.1K, ../../[email protected]/2-0001-Add-pg_get_acl.patch)
download | inline diff:
From 63d39969585f628661110f16605be48bb19899b8 Mon Sep 17 00:00:00 2001
From: Joel Jakobsson <[email protected]>
Date: Wed, 19 Jun 2024 12:31:21 +0200
Subject: [PATCH] Add pg_get_acl() function to get the ACL for a database
object.
This SQL-callable function returns the Access Control List (ACL)
for a database object, specified by catalog OID and object OID.
Related Discussion: https://postgr.es/m/[email protected]
---
doc/src/sgml/func.sgml | 16 +++++++++
src/backend/catalog/objectaddress.c | 43 ++++++++++++++++++++++++
src/include/catalog/pg_proc.dat | 6 ++++
src/test/regress/expected/privileges.out | 7 ++++
src/test/regress/sql/privileges.sql | 2 ++
5 files changed, 74 insertions(+)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 2609269610..fa6fdce517 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -26686,6 +26686,22 @@ SELECT currval(pg_get_serial_sequence('sometable', 'id'));
Undefined objects are identified with <literal>NULL</literal> values.
</para></entry>
</row>
+
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>pg_get_acl</primary>
+ </indexterm>
+ <function>pg_get_acl</function> ( <parameter>classid</parameter> <type>oid</type>, <parameter>objid</parameter> <type>oid</type> )
+ <returnvalue>aclitem[]</returnvalue>
+ </para>
+ <para>
+ Returns the Access Control List (ACL) for a database object,
+ specified by catalog OID and object OID.
+ This function is useful for retrieving and inspecting the privileges associated with database objects.
+ This function returns NULL values for undefined objects.
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 7b536ac6fd..0579b66ab7 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -4362,6 +4362,49 @@ pg_identify_object_as_address(PG_FUNCTION_ARGS)
PG_RETURN_DATUM(HeapTupleGetDatum(htup));
}
+/*
+ * SQL-level callable function to obtain the Access Control List (ACL)
+ * of a specified object, given its catalog OID and object OID.
+ */
+Datum
+pg_get_acl(PG_FUNCTION_ARGS)
+{
+ Oid classId = PG_GETARG_OID(0);
+ Oid objectId = PG_GETARG_OID(1);
+ Oid catalogId = (classId == LargeObjectRelationId) ? LargeObjectMetadataRelationId : classId;
+ AttrNumber Anum_oid = get_object_attnum_oid(catalogId);
+ AttrNumber Anum_acl = get_object_attnum_acl(catalogId);
+ Relation rel;
+ HeapTuple tup;
+ Datum datum;
+ bool isnull;
+
+ /* for "pinned" items in pg_depend, return null */
+ if (!OidIsValid(classId) && !OidIsValid(objectId))
+ PG_RETURN_NULL();
+
+ rel = table_open(catalogId, AccessShareLock);
+
+ tup = get_catalog_object_by_oid(rel, Anum_oid, objectId);
+ if (tup == NULL)
+ elog(ERROR, "cache lookup failed for object %u of catalog \"%s\"",
+ objectId, RelationGetRelationName(rel));
+
+ if (Anum_acl != InvalidAttrNumber)
+ {
+ datum = heap_getattr(tup, Anum_acl, RelationGetDescr(rel), &isnull);
+ if (!isnull)
+ {
+ table_close(rel, AccessShareLock);
+ PG_RETURN_DATUM(datum);
+ }
+ }
+
+ table_close(rel, AccessShareLock);
+ PG_RETURN_NULL();
+}
+
+
/*
* Return a palloc'ed string that describes the type of object that the
* passed address is for.
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 6a5476d3c4..5ab9b11b47 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6362,6 +6362,12 @@
proname => 'pg_describe_object', provolatile => 's', prorettype => 'text',
proargtypes => 'oid oid int4', prosrc => 'pg_describe_object' },
+{ oid => '6347', descr => 'get ACL for SQL object',
+ proname => 'pg_get_acl', provolatile => 's', prorettype => '_aclitem',
+ proargtypes => 'oid oid',
+ proargnames => '{classid,objid}',
+ prosrc => 'pg_get_acl' },
+
{ oid => '3839',
descr => 'get machine-parseable identification of SQL object',
proname => 'pg_identify_object', provolatile => 's', prorettype => 'record',
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index eb4b762ea1..7ada124265 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -219,6 +219,13 @@ GRANT INSERT ON atest2 TO regress_priv_user4 GRANTED BY CURRENT_USER;
GRANT TRUNCATE ON atest2 TO regress_priv_user5 GRANTED BY CURRENT_ROLE;
GRANT TRUNCATE ON atest2 TO regress_priv_user4 GRANTED BY regress_priv_user5; -- error
ERROR: grantor must be current user
+-- test pg_get_acl()
+SELECT pg_get_acl('pg_class'::regclass, 'atest2'::regclass::oid);
+ pg_get_acl
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ {regress_priv_user1=arwdDxtm/regress_priv_user1,regress_priv_user2=r/regress_priv_user1,regress_priv_user3=w/regress_priv_user1,regress_priv_user4=a/regress_priv_user1,regress_priv_user5=D/regress_priv_user1}
+(1 row)
+
SET SESSION AUTHORIZATION regress_priv_user2;
SELECT session_user, current_user;
session_user | current_user
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index eeb4c00292..36f348051e 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -190,6 +190,8 @@ GRANT TRUNCATE ON atest2 TO regress_priv_user5 GRANTED BY CURRENT_ROLE;
GRANT TRUNCATE ON atest2 TO regress_priv_user4 GRANTED BY regress_priv_user5; -- error
+-- test pg_get_acl()
+SELECT pg_get_acl('pg_class'::regclass, 'atest2'::regclass::oid);
SET SESSION AUTHORIZATION regress_priv_user2;
SELECT session_user, current_user;
--
2.45.1
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Add pg_get_acl() function get the ACL for a database object
@ 2024-06-19 14:23 Isaac Morland <[email protected]>
parent: Joel Jacobson <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Isaac Morland @ 2024-06-19 14:23 UTC (permalink / raw)
To: Joel Jacobson <[email protected]>; +Cc: pgsql-hackers
On Wed, 19 Jun 2024 at 07:35, Joel Jacobson <[email protected]> wrote:
> Hello hackers,
>
> Currently, obtaining the Access Control List (ACL) for a database object
> requires querying specific pg_catalog tables directly, where the user
> needs to know the name of the ACL column for the object.
>
I have no idea how often this would be useful, but I wonder if it could
work to have overloaded single-parameter versions for each of regprocedure
(pg_proc.proacl), regclass (pg_class.relacl), …. To call, just cast the OID
to the appropriate reg* type.
For example: To get the ACL for table 'example_table', call pg_get_acl
('example_table'::regclass)
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2024-06-19 14:23 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-02 18:57 [PATCH 4/8] Optimize allocations in bringetbitmap Tomas Vondra <[email protected]>
2024-06-19 11:34 Add pg_get_acl() function get the ACL for a database object Joel Jacobson <[email protected]>
2024-06-19 14:23 ` Re: Add pg_get_acl() function get the ACL for a database object Isaac Morland <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox