The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: tested, failed
Spec compliant: not tested
Documentation: not tested
Hi,
I looked at v10, focused on whether the generated CREATE POLICY statement
can be executed again.
The patch applies cleanly on current master at
8a86aa313a714adc56c74e4b08793e4e6102b5ca.
git diff --check reports no issues.
I built with:
./configure --prefix="$PWD/pg-install" --without-readline --without-zlib
--without-icu
make -s -j8
make -s install
make -C src/test/regress check TESTS=rowsecurity
ended up running the full parallel_schedule in this makefile; all 245 tests
passed, including rowsecurity.
I found one correctness issue in the generated non-pretty DDL. The code
assumes that pg_get_expr_ext(..., false) already returns the parentheses
required by CREATE POLICY syntax, but that is not true for simple boolean
constants.
For example:
CREATE TABLE t(a int);
CREATE POLICY p_true ON t USING (true);
SELECT ddl FROM pg_get_policy_ddl('t', 'p_true', 'pretty', 'false') AS ddl;
returns:
CREATE POLICY p_true ON public.t USING true;
If I drop the policy and execute that generated statement, it fails:
ERROR: syntax error at or near "true"
LINE 1: CREATE POLICY p_true ON public.t USING true;
^
The same issue reproduces for WITH CHECK:
CREATE POLICY p_check ON t FOR INSERT WITH CHECK (false);
is reconstructed as:
CREATE POLICY p_check ON public.t FOR INSERT WITH CHECK false;
and executing it fails at "false".
So I think USING and WITH CHECK need to be parenthesized in non-pretty mode
too, or the tests should include a round-trip execution check for generated
DDL with simple boolean expressions.
I used two small SQL reproducers for the manual checks; the complete repro is
included above.
I have not reviewed the broader pg_get_*_ddl API design or every possible
policy expression form.
Regards,
Ilmar Yunusov
The new status of this patch is: Waiting on Author
