timescaledb-toolkit icon indicating copy to clipboard operation
timescaledb-toolkit copied to clipboard

Using `toolkit_experimental` accessors within function pipelines when stable accessors with the same name also exist

Open rtwalker opened this issue 2 years ago • 0 comments

Note: I'm illustrating the issue through the use of

  • the counter_agg() and toolkit_experimental.gauge_agg() aggregates
  • their respective delta() and toolkit_experimental.delta() accessors
  • the -> operator for function pipelines

I haven't yet enumerated the list of conflicting/accidental (re)use of accessors, so I'm sure how pervasive this behavior is.

When we try to use the toolkit_experimental.delta() on a toolkit_experimental.gauge_agg() as part of a function pipeline, we get an error:

SELECT toolkit_experimental.gauge_agg(ts, val) -> toolkit_experimental.delta() FROM test;
-- ERROR:  function toolkit_experimental.delta() does not exist
-- LINE 1: SELECT toolkit_experimental.gauge_agg(ts, val) -> toolkit_ex...
--                                                           ^
-- HINT:  No function matches the given name and argument types. You might need to add explicit type casts.

This is because while we do define a toolkit_experiemental.delta() function here https://github.com/timescale/timescaledb-toolkit/blob/cf85c41d7e8541f95b3579643cc8298688281f4f/extension/src/gauge_agg.rs#L425-L428

we do not define delta() accessor inside of a toolkit_experimental schema and instead only define a stable delta() accessor here https://github.com/timescale/timescaledb-toolkit/blob/68e8174374568295dcf3dcdb2bf901cdcc4365cb/extension/src/accessors.rs#L77

Nevertheless, because the arrow_delta() function that gets called by -> takes an AccessorDelta as it 2nd argument https://github.com/timescale/timescaledb-toolkit/blob/68e8174374568295dcf3dcdb2bf901cdcc4365cb/extension/src/gauge_agg.rs#L419-L423

we can still use delta() on a gauge agg via function piplelines, but in a way that is perhaps unexpected.

-- this does work; but should it??
SELECT toolkit_experimental.gauge_agg(ts, val) -> delta() AS delta FROM test;

IMO this is at the least weird, if not outright buggy, behavior. Especially when we consider the "normal" use of the accessors in the below queries.

-- this works
SELECT toolkit_experimental.delta(toolkit_experimental.gauge_agg(ts, val)) FROM test;

-- this doesn't work
SELECT delta(toolkit_experimental.gauge_agg(ts, val)) FROM test;

Solutions

A summary of possible approaches to this issue when it was surfaced in the team Slack channel is laid out as such:

  1. We could not implement arrow operators on experimental features
  2. We could require experimental features to use an experimental accessor even when we have a stable corresponding accessor
  3. We could just allow this behavior

I'd argue that Option 1 feels unnecessarily restrictive--we’ve already stabilized -> as a concept and we (now) understand how to avoid this behavior should we choose to.

It has also been pointed out that Option 3 has the potential of becoming confusing in the future if we have a feature with some APIs that happen to match stable accessors and others thats don't.

Assuming Option 2 is possible, I think it makes the most sense. However, a first attempt at doing something like

accessor! { delta() }

#[pg_schema]
pub mod toolkit_experimental {
    pub use pgx::*;
    accessor! { delta() }
}

gave me back error: symbol `__pgx_internals_fn_accessor_delta` is already defined

rtwalker avatar Sep 06 '22 19:09 rtwalker