[pgpool-general: 8302] Re: Timestamp cast not cached
Tatsuo Ishii
ishii at sraoss.co.jp
Tue Jul 5 13:24:06 JST 2022
Oops.
The patch lacked a patch for pool_timestamp.c. Please disregard it.
> Attached is a patch to not cache timestamptz and timetz cast (should
> be applied on top of the previous patch taken from the git repo).
>
>>> Agree but for some reason timestamptz cached and timestamp not.
>>
>> Yes.
>>
>>> I believe it’s happened because the selected cast function for timestamp
>>> has provolatile = “s” and the selected cast function for timestamptz has
>>> provolatile = “i”.
>>>
>>> Both castType have multiple cast function configured. Those functions that
>>> are using time zone has provolatile “s” and those are not using time zone
>>> have provolatile “i”.
>>>
>>> I think in order to fix this issue in a generic way. The solution should
>>> understand which cast function are fetched by postgres when the client ask
>>> for casting.
>>
>> PostgreSQL's parser (and Pgpool-II parser) do not work that way for
>> type cast. The cast is transformed to a type cast node, which does not
>> involve function call at all. So looking into pg_proc is not
>> possible. Probably I can add some codes to handle type cast node to
>> deal with the issue.
>>
>>> Thanks,
>>>
>>> Avi
>>>
>>> On Tue, 5 Jul 2022 at 0:38 Tatsuo Ishii <ishii at sraoss.co.jp> wrote:
>>>
>>>> provolatile column of pg_proc has been already considered in the
>>>> existing code. See the code block started with "if (IsA(node,
>>>> FuncCall))" in non_immutable_function_call_walker().
>>>>
>>>> > I thought the provolatile should be considered. Because I saw in the
>>>> block
>>>> > code you disable today in order to fix the issue, a method which called
>>>> > isSystemType.
>>>> >
>>>> > That function has condition which compare some value with the pg_catalog
>>>> so
>>>> > I thought it’s could be related.
>>>> >
>>>> > Thanks,
>>>> >
>>>> > Avi
>>>> >
>>>> > On Mon, 4 Jul 2022 at 15:34 Tatsuo Ishii <ishii at sraoss.co.jp> wrote:
>>>> >
>>>> >> provolatile column of pg_proc is not involved here. After PostgreSQL
>>>> >> (Pgpool-II) parses "Select ‘2022-02-18
>>>> >> 07:00:00.006547+02’::timestamptz;" it produces a parse tree like
>>>> >> below. As you can see, there's no function call in it. It is
>>>> >> essentially a "type cast" node with type name "timestamptz".
>>>> >>
>>>> >> I think what Pgpool-II needs to do here is, finding a type cast node
>>>> >> with its data type "timestamptz" (probably "timetz" should be
>>>> >> considered as well). If it finds, mark the SQL as "we should not cache
>>>> >> it".
>>>> >>
>>>> >> test=# Select '2022-02-18 07:00:00.006547+02'::timestamptz;
>>>> >> 2022-07-04 20:16:13.571 JST [896725] LOG: raw parse tree:
>>>> >> 2022-07-04 20:16:13.571 JST [896725] DETAIL: (
>>>> >> {RAWSTMT
>>>> >> :stmt
>>>> >> {SELECT
>>>> >> :distinctClause <>
>>>> >> :intoClause <>
>>>> >> :targetList (
>>>> >> {RESTARGET
>>>> >> :name <>
>>>> >> :indirection <>
>>>> >> :val
>>>> >> {TYPECAST
>>>> >> :arg
>>>> >> {A_CONST
>>>> >> :val "\2022-02-18\ 07
>>>> >> :00
>>>> >> :00.006547+02"
>>>> >> :location 7
>>>> >> }
>>>> >> :typeName
>>>> >> {TYPENAME
>>>> >> :names ("timestamptz")
>>>> >> :typeOid 0
>>>> >> :setof false
>>>> >> :pct_type false
>>>> >> :typmods <>
>>>> >> :typemod -1
>>>> >> :arrayBounds <>
>>>> >> :location 40
>>>> >> }
>>>> >> :location 38
>>>> >> }
>>>> >> :location 7
>>>> >> }
>>>> >> )
>>>> >> :fromClause <>
>>>> >> :whereClause <>
>>>> >> :groupClause <>
>>>> >> :groupDistinct false
>>>> >> :havingClause <>
>>>> >> :windowClause <>
>>>> >> :valuesLists <>
>>>> >> :sortClause <>
>>>> >> :limitOffset <>
>>>> >> :limitCount <>
>>>> >> :limitOption 0
>>>> >> :lockingClause <>
>>>> >> :withClause <>
>>>> >> :op 0
>>>> >> :all false
>>>> >> :larg <>
>>>> >> :rarg <>
>>>> >> }
>>>> >> :stmt_location 0
>>>> >> :stmt_len 51
>>>> >> }
>>>> >> )
>>>> >>
>>>> >> Best reagards,
>>>> >> --
>>>> >> Tatsuo Ishii
>>>> >> SRA OSS, Inc. Japan
>>>> >> English: http://www.sraoss.co.jp/index_en.php
>>>> >> Japanese:http://www.sraoss.co.jp
>>>> >>
>>>> >> > Maybe it’s not a big issue.
>>>> >> >
>>>> >> > But more than that the way postgresql using casting function is
>>>> related
>>>> >> to
>>>> >> > the procvolatile.
>>>> >> >
>>>> >> > I believe the solution for pgpool should be related to which casting
>>>> >> > function postgres using.
>>>> >> >
>>>> >> > In case the procvolatile for that cast function is “i” the query
>>>> should
>>>> >> be
>>>> >> > cached and if the procvolatile is “s” it shouldn’t.
>>>> >> >
>>>> >> > But maybe I am wrong.
>>>> >> >
>>>> >> > Do your magic.
>>>> >> >
>>>> >> > Thanks,
>>>> >> >
>>>> >> > Avi
>>>> >> >
>>>> >> > On Mon, 4 Jul 2022 at 14:36 Tatsuo Ishii <ishii at sraoss.co.jp> wrote:
>>>> >> >
>>>> >> >> > Hi,
>>>> >> >> >
>>>> >> >> > I found some issue with timestamptz cast.
>>>> >> >> > See the following:
>>>> >> >> >
>>>> >> >> > 1.
>>>> >> >> >
>>>> >> >> > 2. Select ‘2022-02-18 07:00:00.006547+02’::timestamptz; ―> will
>>>> >> retrieved
>>>> >> >> > from cache
>>>> >> >> >
>>>> >> >> > 3. Set time zone to ‘Some time zone’;
>>>> >> >> >
>>>> >> >> > 4. Select ‘2022-02-18 07:00:00.006547+02’::timestamptz; ―> will
>>>> >> returned
>>>> >> >> > from cache but shouldn’t because the time zone has been changed.
>>>> >> >> >
>>>> >> >> > I think the right behaviour should be that if we using cast which
>>>> >> >> involved
>>>> >> >> > timezone like timestamptz or timetz these queries shouldn’t saved
>>>> in
>>>> >> >> cache.
>>>> >> >> >
>>>> >> >> > What are your thoughts?
>>>> >> >>
>>>> >> >> You are right. Let me think about how to deal with the case.
>>>> >> >>
>>>> >> >> > Thanks,
>>>> >> >> >
>>>> >> >> > Avi.
>>>> >> >> >
>>>> >> >> > On Mon, 4 Jul 2022 at 11:41 Tatsuo Ishii <ishii at sraoss.co.jp>
>>>> wrote:
>>>> >> >> >
>>>> >> >> >> Glad to hear that :-)
>>>> >> >> >>
>>>> >> >> >> > My mistake it’s working like a charm :)
>>>> >> >> >> >
>>>> >> >> >> > On Mon, 4 Jul 2022 at 11:32 Avi Raboah <avi.raboah at gmail.com>
>>>> >> wrote:
>>>> >> >> >> >
>>>> >> >> >> >> I added the patch and it still not working.
>>>> >> >> >> >> After your change the query
>>>> >> >> >> >> Select ‘2022-02-18 07:00:00.006547’::timestamp;
>>>> >> >> >> >>
>>>> >> >> >> >> Still not cached
>>>> >> >> >> >>
>>>> >> >> >> >> On Mon, 4 Jul 2022 at 11:21 Tatsuo Ishii <ishii at sraoss.co.jp>
>>>> >> wrote:
>>>> >> >> >> >>
>>>> >> >> >> >>> Hi,
>>>> >> >> >> >>>
>>>> >> >> >> >>> > Hi,
>>>> >> >> >> >>> >
>>>> >> >> >> >>> > I saw your patch thanks for that.
>>>> >> >> >> >>>
>>>> >> >> >> >>> You are welcome.
>>>> >> >> >> >>>
>>>> >> >> >> >>> > One question, in order to enable the not_used block you add,
>>>> >> Do I
>>>> >> >> >> need
>>>> >> >> >> >>> to
>>>> >> >> >> >>> > define this macro in the same page?
>>>> >> >> >> >>>
>>>> >> >> >> >>> No. You should *not* define NOT_USED symbol. Otherwise, the
>>>> block
>>>> >> >> will
>>>> >> >> >> >>> be enabled, which is opposite to what the patch wants to do.
>>>> >> >> >> >>>
>>>> >> >> >> >>> > For example:
>>>> >> >> >> >>> > #define NOT_USED
>>>> >> >> >> >>> > #ifdef NOT_USED
>>>> >> >> >> >>> > …
>>>> >> >> >> >>> > …
>>>> >> >> >> >>> > #endif
>>>> >> >> >> >>> >
>>>> >> >> >> >>> > Or I don’t need to add that ?
>>>> >> >> >> >>> >
>>>> >> >> >> >>> > Thanks,
>>>> >> >> >> >>> >
>>>> >> >> >> >>> > Avi.
>>>> >> >> >> >>> >
>>>> >> >> >> >>> > On Mon, 4 Jul 2022 at 9:54 Avi Raboah <avi.raboah at gmail.com
>>>> >
>>>> >> >> wrote:
>>>> >> >> >> >>> >
>>>> >> >> >> >>> >> Awesome, thanks!
>>>> >> >> >> >>> >>
>>>> >> >> >> >>> >> On Mon, 4 Jul 2022 at 9:52 Tatsuo Ishii <
>>>> ishii at sraoss.co.jp>
>>>> >> >> wrote:
>>>> >> >> >> >>> >>
>>>> >> >> >> >>> >>> > Thanks a lot for your fast reaponse!
>>>> >> >> >> >>> >>> >
>>>> >> >> >> >>> >>> > Do you know when the fix will be available?
>>>> >> >> >> >>> >>>
>>>> >> >> >> >>> >>> I have just pushed the fix. It will available in the next
>>>> >> >> scheduled
>>>> >> >> >> >>> >>> release (Aug 18).
>>>> >> >> >> >>> >>>
>>>> >> >> >> >>> >>> https://pgpool.net/mediawiki/index.php/Roadmap
>>>> >> >> >> >>> >>>
>>>> >> >> >> >>> >>> If you need patches, you can grab from the git repository.
>>>> >> >> >> >>> >>>
>>>> >> >> >> >>> >>>
>>>> >> https://pgpool.net/mediawiki/index.php/Source_code_repository
>>>> >> >> >> >>> >>>
>>>> >> >> >> >>> >>> Best reagards,
>>>> >> >> >> >>> >>> --
>>>> >> >> >> >>> >>> Tatsuo Ishii
>>>> >> >> >> >>> >>> SRA OSS, Inc. Japan
>>>> >> >> >> >>> >>> English: http://www.sraoss.co.jp/index_en.php
>>>> >> >> >> >>> >>> Japanese:http://www.sraoss.co.jp
>>>> >> >> >> >>> >>>
>>>> >> >> >> >>> >>
>>>> >> >> >> >>>
>>>> >> >> >> >>
>>>> >> >> >>
>>>> >> >>
>>>> >>
>>>>
>> _______________________________________________
>> pgpool-general mailing list
>> pgpool-general at pgpool.net
>> http://www.pgpool.net/mailman/listinfo/pgpool-general
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cache-timestamp.patch
Type: text/x-patch
Size: 1739 bytes
Desc: not available
URL: <http://www.pgpool.net/pipermail/pgpool-general/attachments/20220705/1a061e82/attachment.bin>
More information about the pgpool-general
mailing list