Re: Infinite Interval

On Thu, Sep 21, 2023 at 8:35 AM jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>
> > On Wed, 20 Sept 2023 at 15:00, Ashutosh Bapat
> > <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> > >
> > > 0005 - Refactored Jian's code fixing window functions. Does not
> > > contain the changes for serialization and deserialization. Jian,
> > > please let me know if I have missed anything else.
> > >
>
> attached serialization and deserialization function.
>

Thanks. They look good. I have incorporated those in the attached patch set.

One thing I didn't understand though is the use of
makeIntervalAggState() in interval_avg_deserialize(). In all other
deserialization functions like numeric_avg_deserialize() we create the
Agg State in CurrentMemoryContext but makeIntervalAggState() creates
it in aggcontext. And it works. We could change the code to allocate
agg state in aggcontext. Not a big change. But I did not find any
explanation as to why we use CurrentMemoryContext in other places.
Dean, do you have any idea?

>
> >
> > Also, in do_interval_discard(), this seems a bit risky:
> >
> > + neg_val.day = -newval->day;
> > + neg_val.month = -newval->month;
> > + neg_val.time = -newval->time;
> >
>
> we already have interval negate function, So I changed to interval_um_internal.
> based on 20230920 patches. I have made the attached changes.

I didn't use this since it still requires neg_val variable and the
implementation for finite interval subtraction would still be differ
in interval_mi and do_interval_discard().

>
> The serialization do make big difference when configure to parallel mode.

Yes. On my machine queryE shows following timings, that's huge change
because of parallel query.
with the ser/deser functions: 112.193 ms
without those functions: 272.759 ms.

Before the introduction of Internal IntervalAggState, there were no
serialize, deserialize functions. I wonder how did parallel query
work. Did it just use serialize/deserialize functions of _interval?

The attached patches are thus
0001 - 0005 - same as the last patch set.
Dean, if you are fine with the changes in 0004, I would like to merge
that into 0003.

0006 - uses pg_add/sub_s32/64_overflow functions in finite_interval_pl
and also introduces finite_interval_mi as suggested by Dean.
0007 - implements serialization and deserialization functions, but
uses aggcontext for deser.

Once we are fine with the last three patches, they need to be merged into 0003.

--
Best Wishes,
Ashutosh Bapat