[pgpool-general: 8308] Re: Timestamp cast not cached
Tatsuo Ishii
ishii at sraoss.co.jp
Wed Jul 6 09:36:55 JST 2022
I have posted a question to pgsql-hackers regarding to_timestamp.
https://www.postgresql.org/message-id/flat/20220705.172957.2068967435108479827.t-ishii%40sranhm.sra.co.jp
It turned out that the provolatile value for the two forms of
to_timestamp are correct. Surely to_timestamp(1 argument) returns
different result depending on the time zone but the actual internal
value of timestamptz is identical. The output difference is merely how
the function prints timestamptz according to the time zone.
Here are examples provided by Tom Lane.
regression=# show timezone;
TimeZone
------------------
America/New_York
(1 row)
regression=# select to_timestamp(0);
to_timestamp
------------------------
1969-12-31 19:00:00-05
(1 row)
regression=# set timezone = 'utc';
SET
regression=# select to_timestamp(0);
to_timestamp
------------------------
1970-01-01 00:00:00+00
(1 row)
"1969-12-31 19:00:00-05" and "1970-01-01 00:00:00+00" are actually
same value as timestamptz data type, which means labeling this form of
to_timestamp as immutable is correct.
However this does not solve the problem of query cache in Pgpool-II.
The disccusion above suggests that even if the function is labeled as
immutable, there are cases when pgpool should not create a cache for
the SELECT which uses the function (you already showed a good
example). I thinkk pgpool should not create a cache if a function is
labeled other than immutable (we already do it) and if its return type
is timestamptz or timetz (we have not done it yet).
But there's more.
PostgreSQL has similar config settings that change the output style of
functions/expressions: namely datestyle and client_encoding etc.
test=# show datestyle;
DateStyle
-----------
ISO, MDY
(1 row)
test=# select '2022-07-06'::date;
date
------------
2022-07-06
(1 row)
test=# set datestyle to 'Postgres, mdy';
SET
test=# select '2022-07-06'::date;
date
------------
07-06-2022
(1 row)
If we execute these using pgpool with query cache enabled, we get:
test=# show datestyle;
DateStyle
-----------
ISO, MDY
(1 row)
test=# select '2022-07-06'::date;
date
------------
2022-07-06
(1 row)
test=# set datestyle to 'Postgres, mdy';
SET
test=# select '2022-07-06'::date;
date
------------
2022-07-06
(1 row)
Notice the last result does reflect the datestyle setting change
because of query cache.
I think there are more config parameters that induce the wrong
behavior of pgpool:
IntervalStyle, extra_float_digits, lc_messages, lc_monetary, lc_numeric, lc_time
For now I don't know how to deal with the problem. Maybe we should
just add this as a restriction to the doc?
> Thanks a lot.
>
> Really appreciate it.
>
> Avi
>
>
>
>
> On Tue, 5 Jul 2022 at 11:10 Tatsuo Ishii <ishii at sraoss.co.jp> wrote:
>
>> > So the last patch have the fixed?
>>
>> Yes.
>>
>> > Do you know how postgres decide which cast function to use?
>>
>> I just have a vague image:
>> It's done in the alalyze phase. In my understaing PostgreSQL does it by
>> using pg_cast system catalog plus I/O functions which are defined when
>> the types are defined. See src/backend/parser/analyze.c for more details.
>>
>> > I can see that if you do the following (without the fix from yesterday) -
>> >
>> > Select to_timestamp(150000);
>> >
>> > The above query cached.
>> > Than I check in the pg_proc what function was running and I found that
>> the
>> > function that running is float8_timestamptz
>>
>> No. The function Pgpool-II chose was to_timestamp with 1 argument
>> (there is another to_timestamp function which accepts 2 args) . The
>> reason why to_timestamp(150000) is cached is, pg_proc has wrong
>> provolatile value for the function:
>>
>> select provolatile, proargtypes from pg_proc where proname =
>> 'to_timestamp' and proargtypes[0] = 701;
>> provolatile | proargtypes
>> -------------+-------------
>> i | 701
>> (1 row)
>>
>> As you can see, the provolatile value of the function is 'i'
>> (immutable), which is clearly wrong because if the time zone is
>> changed, the value would be changed too. And Pgpool-II believes that
>> the function result can be cached.
>>
>> I am going to report this to pgsql-hackers.
>>
>> > Thanks,
>> >
>> > Avi.
>> >
>> > On Tue, 5 Jul 2022 at 7:24 Tatsuo Ishii <ishii at sraoss.co.jp> wrote:
>> >
>> >> 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
>> >>
>>
More information about the pgpool-general
mailing list