Fix gapfill bug when aggregates are in expressions with groupby columns

[natalya-aksman]

Fixes #4894

Gapfill used to error out when aggregates were used in expressions with group-by columns like (agg + gby_var) and when gapfill target entries were not matching the table column order.
It is fixed by fixing group-by vars to match execution group-by vars.

[github-actions]

@dbeck, @mkindahl: please review this pull request.

Powered by pull-review

[codecov]

Codecov Report

❌ Patch coverage is 90.90909% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.48%. Comparing base (0843d8b) to head (554622c).
⚠️ Report is 77 commits behind head on main.

Files with missing lines	Patch %	Lines
tsl/src/nodes/gapfill/gapfill_exec.c	85.71%	1 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8550      +/-   ##
==========================================
+ Coverage   82.37%   82.48%   +0.10%     
==========================================
  Files         248      248              
  Lines       47632    47633       +1     
  Branches    12101    12104       +3     
==========================================
+ Hits        39237    39289      +52     
- Misses       3503     3507       +4     
+ Partials     4892     4837      -55     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[akuzm]

One thing I don't understand about this: suppose you have coalesce(x, min(y)) in the output of gapfill. In the gapfilled tuple, you're supposed to compute coalesce(x, null). But how can you compute x based on the gapfilled tuple, if x is not present there at all?

[akuzm]

Looks like this is still not correct:

test=# WITH hourly(time, signal, value, level_a, level_b, level_c, agg) AS (
  VALUES
    ('2022-10-01T00:00:00Z'::timestamptz, 2, 685::real, 1, -1, -1, 2),
    ('2022-10-01T00:00:00Z'::timestamptz, 2, 686::real, 1, -1, -1, 3),
    ('2022-10-01T02:00:00Z'::timestamptz, 2, 686::real, 1, -1, -1, 2),
    ('2022-10-01T02:00:00Z'::timestamptz, 2, 687::real, 1, -1, -1, 3),
    ('2022-10-01T03:00:00Z'::timestamptz, 2, 687::real, 1, -1, -1, 2),
    ('2022-10-01T03:00:00Z'::timestamptz, 2, 688::real, 1, -1, -1, 3)
)
SELECT
  time_bucket_gapfill('1 hour', time) AS time, 1 p1, 2 p2,
  (agg + 1) * max(value)                      
FROM hourly
WHERE
  time >= '2022-10-01T00:00:00Z'
  AND time <  '2022-10-01T05:59:59Z'
GROUP BY 1, agg + 1;
ERROR:  XX000: attribute number 7 exceeds number of columns 5
LOCATION:  CheckVarSlotCompatibility, execExprInterp.c:2015

[natalya-aksman]

One thing I don't understand about this: suppose you have coalesce(x, min(y)) in the output of gapfill. In the gapfilled tuple, you're supposed to compute coalesce(x, null). But how can you compute x based on the gapfilled tuple, if x is not present there at all?

When we have aggregates, the only vars which can be used in the target list are aggregates and grouping columns. Grouping columns are guaranteed to be computed and provided by the aggregate node. I.e. x is guaranteed to be present in the gapfill tuple as a groping column, so we can refer to it in this expression.

[natalya-aksman]

Looks like this is still not correct:
...
FROM hourly
WHERE
time >= '2022-10-01T00:00:00Z'
AND time < '2022-10-01T05:59:59Z'
GROUP BY 1, agg + 1;
ERROR: XX000: attribute number 7 exceeds number of columns 5
LOCATION: CheckVarSlotCompatibility, execExprInterp.c:2015

We need to cover any grouping expressions, not just vars. Will look into it.