[pgpool-hackers: 3729] Re: Query mis cached

[Tatsuo Ishii]

Hi Hou, Zhijie,

I have committed/pushed this. Thank you!

> Hi Ishii san
> 
>> For DELETE and UPDATE statement, I think you need to cast
>> cte->ctequery to DeleteStmt or UpdateStmt accordingly.
> 
> Thanks for reviewing the patch.
> It's my fault, I forgot to check when copying the code.
> 
> I fixed the mistake and made a new patch.
> 
> Best regards,
> houzj
> 
> -----Original Message-----
> From: Tatsuo Ishii [mailto:ishii at sraoss.co.jp] 
> Sent: Friday, July 10, 2020 9:49 AM
> To: Hou, Zhijie/侯 志杰 <houzj.fnst at cn.fujitsu.com>
> Cc: pgpool-hackers at pgpool.net
> Subject: Re: [pgpool-hackers: 3706] Re: Query mis cached
> 
>>> I found the following SQL will be cached, when enabled memory query 
>>> cache,
>>> 
>>> 
>>> 
>>>         "with cte as (insert into test2 values(1) returning *) select * from test2;"
>>> 
>>> 
>>> 
>>> I think the SQL has Data-Modifying Statements in with clause should not be cached.
>>> 
>>> Because Once cached, the Data-Modifying Statements will not be executed again which is not expected.
>> 
>> You are right.
> 
> At the time when query cache was implemented, there's no WITH + DML. That's why we missed that.
> 
>>> It seems to be better to analyze the with clause like the following:
>>> 
>>> 
>>> 
>>> --------For UPDATE/INSERT/DELETE type SQL:
>>> 
>>>                 extract oids from its with clause which will be invalidated.
>>> 
>>> 
>>> 
>>> --------For SELECT type SQL:
>>> 
>>>                 If Data-Modifying Statements in its with clause, the 
>>> select will not be cached,
>>> 
>>>                 and the oids from with clause will be invalidated too.
>>> 
>>> 
>>> 
>>> What do you think?
> 
> Sounds like a correct idea.
> 
>>> 
>>> 
>>> I try to solve the problem and made a patch for the code.
>> 
>> Thank you for the patch! I will look into this.
> 
> In 0002-Update-pool_memqcache.c.patch:
> 
> +		else if(IsA(cte->ctequery, DeleteStmt))
> +		{
> +			InsertStmt *stmt = (InsertStmt *) cte->ctequery;
> +			table = make_table_name_from_rangevar(stmt->relation);
> +		}
> +		else if(IsA(cte->ctequery, UpdateStmt))
> +		{
> +			InsertStmt *stmt = (InsertStmt *) cte->ctequery;
> +			table = make_table_name_from_rangevar(stmt->relation);
> +		}
> 
> For DELETE and UPDATE statement, I think you need to cast
> cte->ctequery to DeleteStmt or UpdateStmt accordingly.
> 
> Best regards,
> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan
> English: http://www.sraoss.co.jp/index_en.php
> Japanese:http://www.sraoss.co.jp
> 
> 
> 
>