Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make CAgg time_bucket catalog table more generic #6624

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

jnidzwetzki
Copy link
Contributor

@jnidzwetzki jnidzwetzki commented Feb 8, 2024

The catalog table continuous_aggs_bucket_function is currently only used for variable bucket sizes. Information about the fixed-size buckets is stored in the table continuous_agg only. This causes some problems (e.g., we have redundant fields for the bucket_size, fixes size buckets with offsets are not supported, ...).

This commit is the first in a row of commits that refactor the catalog for the CAgg time_bucket function. The goals are:

  • Remove the CAgg redundant attributes in the catalog
  • Create an entry in continuous_aggs_bucket_function for all CAggs that use time_bucket

This first commit refactors the continuous_aggs_bucket_function table and prepares it for more generic use. Not all attributes are used yet, but these will change in follow-up PRs.


Should be merged after: #6661
Disable-check: force-changelog-file

@jnidzwetzki jnidzwetzki force-pushed the refactor_cagg_catalog branch 4 times, most recently from afb5194 to a5c1266 Compare February 9, 2024 09:39
Copy link

codecov bot commented Feb 9, 2024

Codecov Report

Attention: 23 lines in your changes are missing coverage. Please review.

Comparison is base (59f50f2) 80.06% compared to head (0713279) 81.51%.
Report is 8 commits behind head on main.

Files Patch % Lines
tsl/src/continuous_aggs/repair.c 77.19% 1 Missing and 12 partials ⚠️
src/ts_catalog/continuous_agg.c 41.17% 0 Missing and 10 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6624      +/-   ##
==========================================
+ Coverage   80.06%   81.51%   +1.44%     
==========================================
  Files         190      190              
  Lines       37181    36378     -803     
  Branches     9450     9452       +2     
==========================================
- Hits        29770    29652     -118     
+ Misses       2997     2969      -28     
+ Partials     4414     3757     -657     

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

@jnidzwetzki jnidzwetzki force-pushed the refactor_cagg_catalog branch 4 times, most recently from 272b43d to 6a4efdd Compare February 9, 2024 11:50
@jnidzwetzki jnidzwetzki marked this pull request as ready for review February 9, 2024 12:07
Copy link

github-actions bot commented Feb 9, 2024

@mahipv, @nikkhils: please review this pull request.

Powered by pull-review

sql/updates/latest-dev.sql Outdated Show resolved Hide resolved
@jnidzwetzki jnidzwetzki force-pushed the refactor_cagg_catalog branch 7 times, most recently from 937ab01 to d25f471 Compare February 14, 2024 14:00
jnidzwetzki added a commit to jnidzwetzki/timescaledb that referenced this pull request Feb 21, 2024
Historically, we have used an empty string for undefined values in the
catalog table continuous_aggs_bucket_function. Since timescale#6624, the optional
arguments can be NULL. This patch cleans up the empty strings and
changes the logic to work with NULL values.
jnidzwetzki added a commit to jnidzwetzki/timescaledb that referenced this pull request Feb 21, 2024
Historically, we have used an empty string for undefined values in the
catalog table continuous_aggs_bucket_function. Since timescale#6624, the optional
arguments can be NULL. This patch cleans up the empty strings and
changes the logic to work with NULL values.
jnidzwetzki added a commit to jnidzwetzki/timescaledb that referenced this pull request Feb 22, 2024
Historically, we have used an empty string for undefined values in the
catalog table continuous_aggs_bucket_function. Since timescale#6624, the optional
arguments can be NULL. This patch cleans up the empty strings and
changes the logic to work with NULL values.
jnidzwetzki added a commit to jnidzwetzki/timescaledb that referenced this pull request Feb 22, 2024
Historically, we have used an empty string for undefined values in the
catalog table continuous_aggs_bucket_function. Since timescale#6624, the optional
arguments can be NULL. This patch cleans up the empty strings and
changes the logic to work with NULL values.
jnidzwetzki added a commit that referenced this pull request Feb 23, 2024
Historically, we have used an empty string for undefined values in the
catalog table continuous_aggs_bucket_function. Since #6624, the optional
arguments can be NULL. This patch cleans up the empty strings and
changes the logic to work with NULL values.
fabriziomello added a commit to fabriziomello/timescaledb that referenced this pull request May 15, 2024
In timescale#6624 we refactored time_bucket catalog table to be more generic and
introduced the `cagg_get_bucket_function` to inspect the query tree of a
given Continuous Aggregate and return the time_bucket function oid.

The problem with the implementation is we traverse the whole query tree
looking for FuncExpr and in certain cases we can have two different
time_bucket function definition but what matters is the correct and
valid time_bucket function that is part of the groupClause.

Fixed it by inspecting only the Query->groupClause items to look for a
valid time_bucket FuncExpr and return it Oid.
fabriziomello added a commit to fabriziomello/timescaledb that referenced this pull request May 15, 2024
In timescale#6624 we refactored time_bucket catalog table to be more generic and
introduced the `cagg_get_bucket_function` to inspect the query tree of
a given Continuous Aggregate and return the time_bucket function oid.

The problem with the implementation is we traverse the whole query tree
looking for `FuncExpr` and in certain cases we can have two different
`time_bucket` function definition but what matters is the correct and
valid `time_bucket` function that is part of the `Query->groupClause`.

Fixed it by inspecting only the `Query->groupClause` items looking for
a valid time bucket `FuncExpr` and return it `Oid`.
fabriziomello added a commit to fabriziomello/timescaledb that referenced this pull request May 15, 2024
In timescale#6624 we refactored time_bucket catalog table to be more generic and
introduced the `cagg_get_bucket_function` to inspect the query tree of
a given Continuous Aggregate and return the time_bucket function oid.

The problem with the implementation is we traverse the whole query tree
looking for `FuncExpr` and in certain cases we can have two different
`time_bucket` function definition but what matters is the correct and
valid `time_bucket` function that is part of the `Query->groupClause`.

Fixed it by inspecting only the `Query->groupClause` items looking for
a valid time bucket `FuncExpr` and return it `Oid`.
fabriziomello added a commit to fabriziomello/timescaledb that referenced this pull request May 15, 2024
In timescale#6624 we refactored time_bucket catalog table to be more generic and
introduced the `cagg_get_bucket_function` to inspect the query tree of
a given Continuous Aggregate and return the time_bucket function oid.

The problem with the implementation is we traverse the whole query tree
looking for `FuncExpr` and in certain cases we can have two different
`time_bucket` function definition but what matters is the correct and
valid `time_bucket` function that is part of the `Query->groupClause`.

Fixed it by inspecting only the `Query->groupClause` items looking for
a valid time bucket `FuncExpr` and return it `Oid`.
fabriziomello added a commit to fabriziomello/timescaledb that referenced this pull request May 15, 2024
In timescale#6624 we refactored time_bucket catalog table to be more generic and
introduced the `cagg_get_bucket_function` to inspect the query tree of
a given Continuous Aggregate and return the time_bucket function oid.

The problem with the implementation is we traverse the whole query tree
looking for `FuncExpr` and in certain cases we can have two different
`time_bucket` function definition but what matters is the correct and
valid `time_bucket` function that is part of the `Query->groupClause`.

Fixed it by inspecting only the `Query->groupClause` items looking for
a valid time bucket `FuncExpr` and return it `Oid`.
fabriziomello added a commit to fabriziomello/timescaledb that referenced this pull request May 16, 2024
In timescale#6624 we refactored time_bucket catalog table to be more generic and
introduced the `cagg_get_bucket_function` to inspect the query tree of
a given Continuous Aggregate and return the time_bucket function oid.

The problem with the implementation is we traverse the whole query tree
looking for `FuncExpr` and in certain cases we can have two different
`time_bucket` function definition but what matters is the correct and
valid `time_bucket` function that is part of the `Query->groupClause`.

Fixed it by inspecting only the `Query->groupClause` items looking for
a valid time bucket `FuncExpr` and return it `Oid`.
fabriziomello added a commit to fabriziomello/timescaledb that referenced this pull request May 17, 2024
In timescale#6624 we refactored time_bucket catalog table to be more generic and
introduced the `cagg_get_bucket_function` to inspect the query tree of
a given Continuous Aggregate and return the time_bucket function oid.

The problem with the implementation is we traverse the whole query tree
looking for `FuncExpr` and in certain cases we can have two different
`time_bucket` function definition but what matters is the correct and
valid `time_bucket` function that is part of the `Query->groupClause`.

Fixed it by inspecting only the `Query->groupClause` items looking for
a valid time bucket `FuncExpr` and return it `Oid`.
fabriziomello added a commit that referenced this pull request May 17, 2024
In #6624 we refactored time_bucket catalog table to be more generic and
introduced the `cagg_get_bucket_function` to inspect the query tree of
a given Continuous Aggregate and return the time_bucket function oid.

The problem with the implementation is we traverse the whole query tree
looking for `FuncExpr` and in certain cases we can have two different
`time_bucket` function definition but what matters is the correct and
valid `time_bucket` function that is part of the `Query->groupClause`.

Fixed it by inspecting only the `Query->groupClause` items looking for
a valid time bucket `FuncExpr` and return it `Oid`.
github-actions bot pushed a commit that referenced this pull request May 17, 2024
In #6624 we refactored time_bucket catalog table to be more generic and
introduced the `cagg_get_bucket_function` to inspect the query tree of
a given Continuous Aggregate and return the time_bucket function oid.

The problem with the implementation is we traverse the whole query tree
looking for `FuncExpr` and in certain cases we can have two different
`time_bucket` function definition but what matters is the correct and
valid `time_bucket` function that is part of the `Query->groupClause`.

Fixed it by inspecting only the `Query->groupClause` items looking for
a valid time bucket `FuncExpr` and return it `Oid`.

(cherry picked from commit 54830d1)
fabriziomello added a commit that referenced this pull request May 17, 2024
In #6624 we refactored time_bucket catalog table to be more generic and
introduced the `cagg_get_bucket_function` to inspect the query tree of
a given Continuous Aggregate and return the time_bucket function oid.

The problem with the implementation is we traverse the whole query tree
looking for `FuncExpr` and in certain cases we can have two different
`time_bucket` function definition but what matters is the correct and
valid `time_bucket` function that is part of the `Query->groupClause`.

Fixed it by inspecting only the `Query->groupClause` items looking for
a valid time bucket `FuncExpr` and return it `Oid`.

(cherry picked from commit 54830d1)
timescale-automation pushed a commit that referenced this pull request May 17, 2024
In #6624 we refactored time_bucket catalog table to be more generic and
introduced the `cagg_get_bucket_function` to inspect the query tree of
a given Continuous Aggregate and return the time_bucket function oid.

The problem with the implementation is we traverse the whole query tree
looking for `FuncExpr` and in certain cases we can have two different
`time_bucket` function definition but what matters is the correct and
valid `time_bucket` function that is part of the `Query->groupClause`.

Fixed it by inspecting only the `Query->groupClause` items looking for
a valid time bucket `FuncExpr` and return it `Oid`.

(cherry picked from commit 54830d1)
fabriziomello added a commit to fabriziomello/timescaledb that referenced this pull request May 21, 2024
In timescale#6624 we refactored the time bucket catalog table to make it more
generic and save information for all Continuous Aggregates. Previously
it stored only variable bucket size information.

The problem is we used the `regprocedure` type to store the OID of the
given time bucket function but unfortunately it is not supported by
`pg_upgrade`.

Fixed it by changing the column to TEXT and resolve to/from OID using
builtin `regprocedurein` and `format_procedure_qualified` functions.

Fixes timescale#6935
fabriziomello added a commit to fabriziomello/timescaledb that referenced this pull request May 21, 2024
In timescale#6624 we refactored the time bucket catalog table to make it more
generic and save information for all Continuous Aggregates. Previously
it stored only variable bucket size information.

The problem is we used the `regprocedure` type to store the OID of the
given time bucket function but unfortunately it is not supported by
`pg_upgrade`.

Fixed it by changing the column to TEXT and resolve to/from OID using
builtin `regprocedurein` and `format_procedure_qualified` functions.

Fixes timescale#6935
fabriziomello added a commit to fabriziomello/timescaledb that referenced this pull request May 21, 2024
In timescale#6624 we refactored the time bucket catalog table to make it more
generic and save information for all Continuous Aggregates. Previously
it stored only variable bucket size information.

The problem is we used the `regprocedure` type to store the OID of the
given time bucket function but unfortunately it is not supported by
`pg_upgrade`.

Fixed it by changing the column to TEXT and resolve to/from OID using
builtin `regprocedurein` and `format_procedure_qualified` functions.

Fixes timescale#6935
fabriziomello added a commit to fabriziomello/timescaledb that referenced this pull request May 21, 2024
In timescale#6624 we refactored the time bucket catalog table to make it more
generic and save information for all Continuous Aggregates. Previously
it stored only variable bucket size information.

The problem is we used the `regprocedure` type to store the OID of the
given time bucket function but unfortunately it is not supported by
`pg_upgrade`.

Fixed it by changing the column to TEXT and resolve to/from OID using
builtin `regprocedurein` and `format_procedure_qualified` functions.

Fixes timescale#6935
fabriziomello added a commit to fabriziomello/timescaledb that referenced this pull request May 22, 2024
In timescale#6624 we refactored the time bucket catalog table to make it more
generic and save information for all Continuous Aggregates. Previously
it stored only variable bucket size information.

The problem is we used the `regprocedure` type to store the OID of the
given time bucket function but unfortunately it is not supported by
`pg_upgrade`.

Fixed it by changing the column to TEXT and resolve to/from OID using
builtin `regprocedurein` and `format_procedure_qualified` functions.

Fixes timescale#6935
fabriziomello added a commit that referenced this pull request May 22, 2024
In #6624 we refactored the time bucket catalog table to make it more
generic and save information for all Continuous Aggregates. Previously
it stored only variable bucket size information.

The problem is we used the `regprocedure` type to store the OID of the
given time bucket function but unfortunately it is not supported by
`pg_upgrade`.

Fixed it by changing the column to TEXT and resolve to/from OID using
builtin `regprocedurein` and `format_procedure_qualified` functions.

Fixes #6935
github-actions bot pushed a commit that referenced this pull request May 22, 2024
In #6624 we refactored the time bucket catalog table to make it more
generic and save information for all Continuous Aggregates. Previously
it stored only variable bucket size information.

The problem is we used the `regprocedure` type to store the OID of the
given time bucket function but unfortunately it is not supported by
`pg_upgrade`.

Fixed it by changing the column to TEXT and resolve to/from OID using
builtin `regprocedurein` and `format_procedure_qualified` functions.

Fixes #6935

(cherry picked from commit 8b994c7)
fabriziomello added a commit that referenced this pull request May 22, 2024
In #6624 we refactored the time bucket catalog table to make it more
generic and save information for all Continuous Aggregates. Previously
it stored only variable bucket size information.

The problem is we used the `regprocedure` type to store the OID of the
given time bucket function but unfortunately it is not supported by
`pg_upgrade`.

Fixed it by changing the column to TEXT and resolve to/from OID using
builtin `regprocedurein` and `format_procedure_qualified` functions.

Fixes #6935

(cherry picked from commit 8b994c7)
fabriziomello added a commit that referenced this pull request May 22, 2024
In #6624 we refactored the time bucket catalog table to make it more
generic and save information for all Continuous Aggregates. Previously
it stored only variable bucket size information.

The problem is we used the `regprocedure` type to store the OID of the
given time bucket function but unfortunately it is not supported by
`pg_upgrade`.

Fixed it by changing the column to TEXT and resolve to/from OID using
builtin `regprocedurein` and `format_procedure_qualified` functions.

Fixes #6935

(cherry picked from commit 8b994c7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants