Thread

Commits

Rename the role in stats_ext to have regress_ prefix
- 6963e723f1dd 10.19 landed
- bce24d1edbcf 11.14 landed
- 5f8dd5dc1701 12.9 landed
- c8213aa94962 13.5 landed
- 4090ff2a99b7 14.0 landed
- 628bc9d13be8 15.0 landed
Fix lookup error in extended stats ownership check
- bfb732c0e849 10.19 landed
- b46ff4b50f44 11.14 landed
- 6c8b98669d14 12.9 landed
- 1fe1a04af81d 13.5 landed
- a371a5ba349d 14.0 landed
- 5be8ce82e84f 15.0 landed

Bug fix for cache lookup failure for statistic_ext type

Mark Dilger <mark.dilger@enterprisedb.com> — 2021-08-04T22:03:57Z

Hackers,

You can easily get a cache lookup failure by changing the regression tests as included in this small patch.  The failure looks thus:

+COMMENT ON STATISTICS ab1_a_b_stats IS 'new comment';
+CREATE ROLE temp_role;
+SET SESSION AUTHORIZATION temp_role;
+COMMENT ON STATISTICS ab1_a_b_stats IS 'changed comment';
+ERROR:  cache lookup failed for type 27447
+DROP STATISTICS ab1_a_b_stats;
+ERROR:  cache lookup failed for type 27447
+ALTER STATISTICS ab1_a_b_stats RENAME TO ab1_a_b_stats_new;
+ERROR:  must be owner of statistics object ab1_a_b_stats
+RESET SESSION AUTHORIZATION;
+DROP ROLE temp_role;

I believe this case simply has not had any test coverage, as I don't see any way the current code would ever work.  It treats the Oid of the statistics object as a type, which it is not.

Re: Bug fix for cache lookup failure for statistic_ext type

Tomas Vondra <tomas.vondra@enterprisedb.com> — 2021-08-04T22:32:33Z

On 8/5/21 12:03 AM, Mark Dilger wrote:
> Hackers,
> 
> You can easily get a cache lookup failure by changing the regression tests as included in this small patch.  The failure looks thus:
> 
> +COMMENT ON STATISTICS ab1_a_b_stats IS 'new comment';
> +CREATE ROLE temp_role;
> +SET SESSION AUTHORIZATION temp_role;
> +COMMENT ON STATISTICS ab1_a_b_stats IS 'changed comment';
> +ERROR:  cache lookup failed for type 27447
> +DROP STATISTICS ab1_a_b_stats;
> +ERROR:  cache lookup failed for type 27447
> +ALTER STATISTICS ab1_a_b_stats RENAME TO ab1_a_b_stats_new;
> +ERROR:  must be owner of statistics object ab1_a_b_stats
> +RESET SESSION AUTHORIZATION;
> +DROP ROLE temp_role;
> 
> I believe this case simply has not had any test coverage, as I don't
> see any way the current code would ever work.  It treats the Oid of the
> statistics object as a type, which it is not.
> 

Yeah, you're right - this is broken since 7b504eb282c. Thanks for the 
fix, I'll get it committed.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: Bug fix for cache lookup failure for statistic_ext type

Tomas Vondra <tomas.vondra@enterprisedb.com> — 2021-08-31T17:50:58Z


On 8/5/21 12:32 AM, Tomas Vondra wrote:
> On 8/5/21 12:03 AM, Mark Dilger wrote:
>> Hackers,
>>
>> You can easily get a cache lookup failure by changing the regression 
>> tests as included in this small patch.  The failure looks thus:
>>
>> +COMMENT ON STATISTICS ab1_a_b_stats IS 'new comment';
>> +CREATE ROLE temp_role;
>> +SET SESSION AUTHORIZATION temp_role;
>> +COMMENT ON STATISTICS ab1_a_b_stats IS 'changed comment';
>> +ERROR:  cache lookup failed for type 27447
>> +DROP STATISTICS ab1_a_b_stats;
>> +ERROR:  cache lookup failed for type 27447
>> +ALTER STATISTICS ab1_a_b_stats RENAME TO ab1_a_b_stats_new;
>> +ERROR:  must be owner of statistics object ab1_a_b_stats
>> +RESET SESSION AUTHORIZATION;
>> +DROP ROLE temp_role;
>>
>> I believe this case simply has not had any test coverage, as I don't
>> see any way the current code would ever work.  It treats the Oid of the
>> statistics object as a type, which it is not.
>>
> 
> Yeah, you're right - this is broken since 7b504eb282c. Thanks for the 
> fix, I'll get it committed.
> 

I've pushed a fix for this. And then a fix for the fix :-( because I 
forgot about the rule that role names in regression tests should start 
with regress_ prefix, so animals enforcing this failed.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: Bug fix for cache lookup failure for statistic_ext type

Mark Dilger <mark.dilger@enterprisedb.com> — 2021-08-31T18:53:32Z


> On Aug 31, 2021, at 10:50 AM, Tomas Vondra <tomas.vondra@enterprisedb.com> wrote:
> 
> I've pushed a fix for this. And then a fix for the fix :-( because I forgot about the rule that role names in regression tests should start with regress_ prefix, so animals enforcing this failed.

Thanks!

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company