Re: SYSTEM_USER reserved word implementation

[Jacob Champion]

On Fri, Aug 12, 2022 at 6:32 AM Drouvot, Bertrand <bdrou...@amazon.com> wrote:
> It has been agreed that the work on this patch is on hold until the
> ClientConnectionInfo related work is finished (see the discussion in [1]).
>
> Having said that I'm attaching a new patch
> "v2-0004-system_user-implementation.patch" for the SYSTEM_USER.

(Not a full review.) Now that the implementation has increased in
complexity, the original tests for the parallel workers have become
underpowered. As a concrete example, I forgot to serialize auth_method
during my most recent rewrite, and the tests still passed.

I think it'd be better to check the contents of SYSTEM_USER, where we
can, rather than only testing for existence. Something like the
attached, maybe? And it would also be good to add a similar test to
the authentication suite, so that you don't have to have Kerberos
enabled to fully test SYSTEM_USER.

--Jacob

commit adaff75cb96ec842d15e15df2ee42dd4b3fa1349
Author: Jacob Champion <jchamp...@timescale.com>
Date:   Tue Aug 16 09:32:45 2022 -0700

    squash! SYSTEM_USER implementation
    
    Check the contents of SYSTEM_USER in the Kerberos tests, not just its
    existence.

diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
index 7ce3b33da6..be0dd7c62d 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -178,11 +178,11 @@ $node->safe_psql('postgres', 'CREATE USER test1;');
 
 # Set up a table for SYSTEM_USER parallel worker testing.
 $node->safe_psql('postgres',
-    'CREATE TABLE nulls (n) AS SELECT NULL FROM generate_series(1, 200000);'
+    "CREATE TABLE ids (id) AS SELECT 'gss:test1\@$realm' FROM 
generate_series(1, 200000);"
 );
 
 $node->safe_psql('postgres',
-    'GRANT SELECT ON nulls TO public;'
+    'GRANT SELECT ON ids TO public;'
 );
 
 note "running tests";
@@ -333,7 +333,7 @@ test_query(
        . "SET parallel_setup_cost TO 0;\n"
        . "SET parallel_tuple_cost TO 0;\n"
        . "SET max_parallel_workers_per_gather TO 2;\n"
-       . "SELECT bool_and(SYSTEM_USER IS DISTINCT FROM n) FROM nulls;",
+       . "SELECT bool_and(SYSTEM_USER = id) FROM ids;",
        qr/^t$/s,
        'gssencmode=require',
        'testing system_user with parallel workers');