<div dir="auto">Thanks a lot.</div><div dir="auto"><br></div><div dir="auto">Really appreciate it.</div><div dir="auto"><br></div><div dir="auto">Avi</div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><br></div><div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 5 Jul 2022 at 11:10 Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp">ishii@sraoss.co.jp</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;padding-left:1ex;border-left-color:rgb(204,204,204)">> So the last patch have the fixed?<br>
<br>
Yes.<br>
<br>
> Do you know how postgres decide which cast function to use?<br>
<br>
I just have a vague image:<br>
It's done in the alalyze phase. In my understaing PostgreSQL does it by<br>
using pg_cast system catalog plus I/O functions which are defined when<br>
the types are defined. See src/backend/parser/analyze.c for more details.<br>
<br>
> I can see that if you do the following (without the fix from yesterday) -<br>
> <br>
> Select to_timestamp(150000);<br>
> <br>
> The above query cached.<br>
> Than I check in the pg_proc what function was running and I found that the<br>
> function that running is float8_timestamptz<br>
<br>
No. The function Pgpool-II chose was to_timestamp with 1 argument<br>
(there is another to_timestamp function which accepts 2 args) . The<br>
reason why to_timestamp(150000) is cached is, pg_proc has wrong<br>
provolatile value for the function:<br>
<br>
select provolatile, proargtypes from pg_proc where proname = 'to_timestamp' and proargtypes[0] = 701;<br>
provolatile | proargtypes <br>
-------------+-------------<br>
i | 701<br>
(1 row)<br>
<br>
As you can see, the provolatile value of the function is 'i'<br>
(immutable), which is clearly wrong because if the time zone is<br>
changed, the value would be changed too. And Pgpool-II believes that<br>
the function result can be cached.<br>
<br>
I am going to report this to pgsql-hackers.<br>
<br>
> Thanks,<br>
> <br>
> Avi.<br>
> <br>
> On Tue, 5 Jul 2022 at 7:24 Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>> wrote:<br>
> <br>
>> Oops.<br>
>><br>
>> The patch lacked a patch for pool_timestamp.c. Please disregard it.<br>
>><br>
>> > Attached is a patch to not cache timestamptz and timetz cast (should<br>
>> > be applied on top of the previous patch taken from the git repo).<br>
>> ><br>
>> >>> Agree but for some reason timestamptz cached and timestamp not.<br>
>> >><br>
>> >> Yes.<br>
>> >><br>
>> >>> I believe it’s happened because the selected cast function for<br>
>> timestamp<br>
>> >>> has provolatile = “s” and the selected cast function for timestamptz<br>
>> has<br>
>> >>> provolatile = “i”.<br>
>> >>><br>
>> >>> Both castType have multiple cast function configured. Those functions<br>
>> that<br>
>> >>> are using time zone has provolatile “s” and those are not using time<br>
>> zone<br>
>> >>> have provolatile “i”.<br>
>> >>><br>
>> >>> I think in order to fix this issue in a generic way. The solution<br>
>> should<br>
>> >>> understand which cast function are fetched by postgres when the client<br>
>> ask<br>
>> >>> for casting.<br>
>> >><br>
>> >> PostgreSQL's parser (and Pgpool-II parser) do not work that way for<br>
>> >> type cast. The cast is transformed to a type cast node, which does not<br>
>> >> involve function call at all. So looking into pg_proc is not<br>
>> >> possible. Probably I can add some codes to handle type cast node to<br>
>> >> deal with the issue.<br>
>> >><br>
>> >>> Thanks,<br>
>> >>><br>
>> >>> Avi<br>
>> >>><br>
>> >>> On Tue, 5 Jul 2022 at 0:38 Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>> wrote:<br>
>> >>><br>
>> >>>> provolatile column of pg_proc has been already considered in the<br>
>> >>>> existing code. See the code block started with "if (IsA(node,<br>
>> >>>> FuncCall))" in non_immutable_function_call_walker().<br>
>> >>>><br>
>> >>>> > I thought the provolatile should be considered. Because I saw in<br>
>> the<br>
>> >>>> block<br>
>> >>>> > code you disable today in order to fix the issue, a method which<br>
>> called<br>
>> >>>> > isSystemType.<br>
>> >>>> ><br>
>> >>>> > That function has condition which compare some value with the<br>
>> pg_catalog<br>
>> >>>> so<br>
>> >>>> > I thought it’s could be related.<br>
>> >>>> ><br>
>> >>>> > Thanks,<br>
>> >>>> ><br>
>> >>>> > Avi<br>
>> >>>> ><br>
>> >>>> > On Mon, 4 Jul 2022 at 15:34 Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>><br>
>> wrote:<br>
>> >>>> ><br>
>> >>>> >> provolatile column of pg_proc is not involved here. After<br>
>> PostgreSQL<br>
>> >>>> >> (Pgpool-II) parses "Select ‘2022-02-18<br>
>> >>>> >> 07:00:00.006547+02’::timestamptz;" it produces a parse tree like<br>
>> >>>> >> below. As you can see, there's no function call in it. It is<br>
>> >>>> >> essentially a "type cast" node with type name "timestamptz".<br>
>> >>>> >><br>
>> >>>> >> I think what Pgpool-II needs to do here is, finding a type cast<br>
>> node<br>
>> >>>> >> with its data type "timestamptz" (probably "timetz" should be<br>
>> >>>> >> considered as well). If it finds, mark the SQL as "we should not<br>
>> cache<br>
>> >>>> >> it".<br>
>> >>>> >><br>
>> >>>> >> test=# Select '2022-02-18 07:00:00.006547+02'::timestamptz;<br>
>> >>>> >> 2022-07-04 20:16:13.571 JST [896725] LOG: raw parse tree:<br>
>> >>>> >> 2022-07-04 20:16:13.571 JST [896725] DETAIL: (<br>
>> >>>> >> {RAWSTMT<br>
>> >>>> >> :stmt<br>
>> >>>> >> {SELECT<br>
>> >>>> >> :distinctClause <><br>
>> >>>> >> :intoClause <><br>
>> >>>> >> :targetList (<br>
>> >>>> >> {RESTARGET<br>
>> >>>> >> :name <><br>
>> >>>> >> :indirection <><br>
>> >>>> >> :val<br>
>> >>>> >> {TYPECAST<br>
>> >>>> >> :arg<br>
>> >>>> >> {A_CONST<br>
>> >>>> >> :val "\2022-02-18\ 07<br>
>> >>>> >> :00<br>
>> >>>> >> :00.006547+02"<br>
>> >>>> >> :location 7<br>
>> >>>> >> }<br>
>> >>>> >> :typeName<br>
>> >>>> >> {TYPENAME<br>
>> >>>> >> :names ("timestamptz")<br>
>> >>>> >> :typeOid 0<br>
>> >>>> >> :setof false<br>
>> >>>> >> :pct_type false<br>
>> >>>> >> :typmods <><br>
>> >>>> >> :typemod -1<br>
>> >>>> >> :arrayBounds <><br>
>> >>>> >> :location 40<br>
>> >>>> >> }<br>
>> >>>> >> :location 38<br>
>> >>>> >> }<br>
>> >>>> >> :location 7<br>
>> >>>> >> }<br>
>> >>>> >> )<br>
>> >>>> >> :fromClause <><br>
>> >>>> >> :whereClause <><br>
>> >>>> >> :groupClause <><br>
>> >>>> >> :groupDistinct false<br>
>> >>>> >> :havingClause <><br>
>> >>>> >> :windowClause <><br>
>> >>>> >> :valuesLists <><br>
>> >>>> >> :sortClause <><br>
>> >>>> >> :limitOffset <><br>
>> >>>> >> :limitCount <><br>
>> >>>> >> :limitOption 0<br>
>> >>>> >> :lockingClause <><br>
>> >>>> >> :withClause <><br>
>> >>>> >> :op 0<br>
>> >>>> >> :all false<br>
>> >>>> >> :larg <><br>
>> >>>> >> :rarg <><br>
>> >>>> >> }<br>
>> >>>> >> :stmt_location 0<br>
>> >>>> >> :stmt_len 51<br>
>> >>>> >> }<br>
>> >>>> >> )<br>
>> >>>> >><br>
>> >>>> >> Best reagards,<br>
>> >>>> >> --<br>
>> >>>> >> Tatsuo Ishii<br>
>> >>>> >> SRA OSS, Inc. Japan<br>
>> >>>> >> English: <a href="http://www.sraoss.co.jp/index_en.php" rel="noreferrer" target="_blank">http://www.sraoss.co.jp/index_en.php</a><br>
>> >>>> >> Japanese:<a href="http://www.sraoss.co.jp" rel="noreferrer" target="_blank">http://www.sraoss.co.jp</a><br>
>> >>>> >><br>
>> >>>> >> > Maybe it’s not a big issue.<br>
>> >>>> >> ><br>
>> >>>> >> > But more than that the way postgresql using casting function is<br>
>> >>>> related<br>
>> >>>> >> to<br>
>> >>>> >> > the procvolatile.<br>
>> >>>> >> ><br>
>> >>>> >> > I believe the solution for pgpool should be related to which<br>
>> casting<br>
>> >>>> >> > function postgres using.<br>
>> >>>> >> ><br>
>> >>>> >> > In case the procvolatile for that cast function is “i” the query<br>
>> >>>> should<br>
>> >>>> >> be<br>
>> >>>> >> > cached and if the procvolatile is “s” it shouldn’t.<br>
>> >>>> >> ><br>
>> >>>> >> > But maybe I am wrong.<br>
>> >>>> >> ><br>
>> >>>> >> > Do your magic.<br>
>> >>>> >> ><br>
>> >>>> >> > Thanks,<br>
>> >>>> >> ><br>
>> >>>> >> > Avi<br>
>> >>>> >> ><br>
>> >>>> >> > On Mon, 4 Jul 2022 at 14:36 Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>><br>
>> wrote:<br>
>> >>>> >> ><br>
>> >>>> >> >> > Hi,<br>
>> >>>> >> >> ><br>
>> >>>> >> >> > I found some issue with timestamptz cast.<br>
>> >>>> >> >> > See the following:<br>
>> >>>> >> >> ><br>
>> >>>> >> >> > 1.<br>
>> >>>> >> >> ><br>
>> >>>> >> >> > 2. Select ‘2022-02-18 07:00:00.006547+02’::timestamptz; ―><br>
>> will<br>
>> >>>> >> retrieved<br>
>> >>>> >> >> > from cache<br>
>> >>>> >> >> ><br>
>> >>>> >> >> > 3. Set time zone to ‘Some time zone’;<br>
>> >>>> >> >> ><br>
>> >>>> >> >> > 4. Select ‘2022-02-18 07:00:00.006547+02’::timestamptz; ―><br>
>> will<br>
>> >>>> >> returned<br>
>> >>>> >> >> > from cache but shouldn’t because the time zone has been<br>
>> changed.<br>
>> >>>> >> >> ><br>
>> >>>> >> >> > I think the right behaviour should be that if we using cast<br>
>> which<br>
>> >>>> >> >> involved<br>
>> >>>> >> >> > timezone like timestamptz or timetz these queries shouldn’t<br>
>> saved<br>
>> >>>> in<br>
>> >>>> >> >> cache.<br>
>> >>>> >> >> ><br>
>> >>>> >> >> > What are your thoughts?<br>
>> >>>> >> >><br>
>> >>>> >> >> You are right. Let me think about how to deal with the case.<br>
>> >>>> >> >><br>
>> >>>> >> >> > Thanks,<br>
>> >>>> >> >> ><br>
>> >>>> >> >> > Avi.<br>
>> >>>> >> >> ><br>
>> >>>> >> >> > On Mon, 4 Jul 2022 at 11:41 Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>><br>
>> >>>> wrote:<br>
>> >>>> >> >> ><br>
>> >>>> >> >> >> Glad to hear that :-)<br>
>> >>>> >> >> >><br>
>> >>>> >> >> >> > My mistake it’s working like a charm :)<br>
>> >>>> >> >> >> ><br>
>> >>>> >> >> >> > On Mon, 4 Jul 2022 at 11:32 Avi Raboah <<br>
>> <a href="mailto:avi.raboah@gmail.com" target="_blank">avi.raboah@gmail.com</a>><br>
>> >>>> >> wrote:<br>
>> >>>> >> >> >> ><br>
>> >>>> >> >> >> >> I added the patch and it still not working.<br>
>> >>>> >> >> >> >> After your change the query<br>
>> >>>> >> >> >> >> Select ‘2022-02-18 07:00:00.006547’::timestamp;<br>
>> >>>> >> >> >> >><br>
>> >>>> >> >> >> >> Still not cached<br>
>> >>>> >> >> >> >><br>
>> >>>> >> >> >> >> On Mon, 4 Jul 2022 at 11:21 Tatsuo Ishii <<br>
>> <a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>><br>
>> >>>> >> wrote:<br>
>> >>>> >> >> >> >><br>
>> >>>> >> >> >> >>> Hi,<br>
>> >>>> >> >> >> >>><br>
>> >>>> >> >> >> >>> > Hi,<br>
>> >>>> >> >> >> >>> ><br>
>> >>>> >> >> >> >>> > I saw your patch thanks for that.<br>
>> >>>> >> >> >> >>><br>
>> >>>> >> >> >> >>> You are welcome.<br>
>> >>>> >> >> >> >>><br>
>> >>>> >> >> >> >>> > One question, in order to enable the not_used block<br>
>> you add,<br>
>> >>>> >> Do I<br>
>> >>>> >> >> >> need<br>
>> >>>> >> >> >> >>> to<br>
>> >>>> >> >> >> >>> > define this macro in the same page?<br>
>> >>>> >> >> >> >>><br>
>> >>>> >> >> >> >>> No. You should *not* define NOT_USED symbol. Otherwise,<br>
>> the<br>
>> >>>> block<br>
>> >>>> >> >> will<br>
>> >>>> >> >> >> >>> be enabled, which is opposite to what the patch wants to<br>
>> do.<br>
>> >>>> >> >> >> >>><br>
>> >>>> >> >> >> >>> > For example:<br>
>> >>>> >> >> >> >>> > #define NOT_USED<br>
>> >>>> >> >> >> >>> > #ifdef NOT_USED<br>
>> >>>> >> >> >> >>> > …<br>
>> >>>> >> >> >> >>> > …<br>
>> >>>> >> >> >> >>> > #endif<br>
>> >>>> >> >> >> >>> ><br>
>> >>>> >> >> >> >>> > Or I don’t need to add that ?<br>
>> >>>> >> >> >> >>> ><br>
>> >>>> >> >> >> >>> > Thanks,<br>
>> >>>> >> >> >> >>> ><br>
>> >>>> >> >> >> >>> > Avi.<br>
>> >>>> >> >> >> >>> ><br>
>> >>>> >> >> >> >>> > On Mon, 4 Jul 2022 at 9:54 Avi Raboah <<br>
>> <a href="mailto:avi.raboah@gmail.com" target="_blank">avi.raboah@gmail.com</a><br>
>> >>>> ><br>
>> >>>> >> >> wrote:<br>
>> >>>> >> >> >> >>> ><br>
>> >>>> >> >> >> >>> >> Awesome, thanks!<br>
>> >>>> >> >> >> >>> >><br>
>> >>>> >> >> >> >>> >> On Mon, 4 Jul 2022 at 9:52 Tatsuo Ishii <<br>
>> >>>> <a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>><br>
>> >>>> >> >> wrote:<br>
>> >>>> >> >> >> >>> >><br>
>> >>>> >> >> >> >>> >>> > Thanks a lot for your fast reaponse!<br>
>> >>>> >> >> >> >>> >>> ><br>
>> >>>> >> >> >> >>> >>> > Do you know when the fix will be available?<br>
>> >>>> >> >> >> >>> >>><br>
>> >>>> >> >> >> >>> >>> I have just pushed the fix. It will available in the<br>
>> next<br>
>> >>>> >> >> scheduled<br>
>> >>>> >> >> >> >>> >>> release (Aug 18).<br>
>> >>>> >> >> >> >>> >>><br>
>> >>>> >> >> >> >>> >>> <a href="https://pgpool.net/mediawiki/index.php/Roadmap" rel="noreferrer" target="_blank">https://pgpool.net/mediawiki/index.php/Roadmap</a><br>
>> >>>> >> >> >> >>> >>><br>
>> >>>> >> >> >> >>> >>> If you need patches, you can grab from the git<br>
>> repository.<br>
>> >>>> >> >> >> >>> >>><br>
>> >>>> >> >> >> >>> >>><br>
>> >>>> >> <a href="https://pgpool.net/mediawiki/index.php/Source_code_repository" rel="noreferrer" target="_blank">https://pgpool.net/mediawiki/index.php/Source_code_repository</a><br>
>> >>>> >> >> >> >>> >>><br>
>> >>>> >> >> >> >>> >>> Best reagards,<br>
>> >>>> >> >> >> >>> >>> --<br>
>> >>>> >> >> >> >>> >>> Tatsuo Ishii<br>
>> >>>> >> >> >> >>> >>> SRA OSS, Inc. Japan<br>
>> >>>> >> >> >> >>> >>> English: <a href="http://www.sraoss.co.jp/index_en.php" rel="noreferrer" target="_blank">http://www.sraoss.co.jp/index_en.php</a><br>
>> >>>> >> >> >> >>> >>> Japanese:<a href="http://www.sraoss.co.jp" rel="noreferrer" target="_blank">http://www.sraoss.co.jp</a><br>
>> >>>> >> >> >> >>> >>><br>
>> >>>> >> >> >> >>> >><br>
>> >>>> >> >> >> >>><br>
>> >>>> >> >> >> >><br>
>> >>>> >> >> >><br>
>> >>>> >> >><br>
>> >>>> >><br>
>> >>>><br>
>> >> _______________________________________________<br>
>> >> pgpool-general mailing list<br>
>> >> <a href="mailto:pgpool-general@pgpool.net" target="_blank">pgpool-general@pgpool.net</a><br>
>> >> <a href="http://www.pgpool.net/mailman/listinfo/pgpool-general" rel="noreferrer" target="_blank">http://www.pgpool.net/mailman/listinfo/pgpool-general</a><br>
>><br>
</blockquote></div></div>