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

Fix ANALYZE crash with custom types #3745

Merged
merged 1 commit into from
Oct 27, 2021

Conversation

nikkhils
Copy link
Contributor

Extensions like PostGIS introduce custom datatypes for columns. They
also have custom statistics for such custom types. We already state in
our comments that stats for custom types are not supported but it was
never tested. Fix that now.

Fixes #3733

@nikkhils nikkhils self-assigned this Oct 27, 2021
@nikkhils nikkhils requested a review from a team as a code owner October 27, 2021 06:11
@nikkhils nikkhils requested review from berkley, svenklemm, duncan-tsdb, pmwkaa and erimatnor and removed request for a team and berkley October 27, 2021 06:11
@codecov
Copy link

codecov bot commented Oct 27, 2021

Codecov Report

Merging #3745 (e4a174e) into master (0fecefd) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3745      +/-   ##
==========================================
- Coverage   90.29%   90.29%   -0.01%     
==========================================
  Files         213      213              
  Lines       37655    37654       -1     
==========================================
- Hits        34001    33999       -2     
- Misses       3654     3655       +1     
Impacted Files Coverage Δ
tsl/src/chunk_api.c 94.88% <100.00%> (ø)
tsl/src/nodes/data_node_dispatch.c 97.10% <0.00%> (-0.25%) ⬇️
tsl/src/bgw_policy/job.c 87.50% <0.00%> (-0.06%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0fecefd...e4a174e. Read the comment docs.

Copy link
Member

@svenklemm svenklemm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add changelog entry

Copy link
Contributor

@erimatnor erimatnor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to add a test by creating a custom type (CREATE TYPE) and then use it in a distributed hypertable that we ANALYZE?

@nikkhils
Copy link
Contributor Author

Is it possible to add a test by creating a custom type (CREATE TYPE) and then use it in a distributed hypertable that we ANALYZE?

No. For this to happen, we need a custom STATISTIC type, not just custom type and that can happen only via PostGIS or other external components.

Extensions like PostGIS introduce custom datatypes for columns. They
also have custom statistics for such custom types. We already state in
our comments that stats for custom types are not supported but it was
never tested. Fix that now.

Fixes timescale#3733
@nikkhils
Copy link
Contributor Author

Please add changelog entry

Done. Thanks

@pmwkaa
Copy link
Contributor

pmwkaa commented Oct 27, 2021

Is it possible to add a test by creating a custom type (CREATE TYPE) and then use it in a distributed hypertable that we ANALYZE?

No. For this to happen, we need a custom STATISTIC type, not just custom type and that can happen only via PostGIS or other external components.

I guess we could introduce such a type in the C part of testing, unless it is unnecessary complicated

@erimatnor
Copy link
Contributor

Is it possible to add a test by creating a custom type (CREATE TYPE) and then use it in a distributed hypertable that we ANALYZE?

No. For this to happen, we need a custom STATISTIC type, not just custom type and that can happen only via PostGIS or other external components.

Isn't it possible to just alias an existing type and reference its stats function? Similar to this:

CREATE OR REPLACE FUNCTION customtype_in(cstring) RETURNS customtype AS

@nikkhils
Copy link
Contributor Author

Is it possible to add a test by creating a custom type (CREATE TYPE) and then use it in a distributed hypertable that we ANALYZE?

No. For this to happen, we need a custom STATISTIC type, not just custom type and that can happen only via PostGIS or other external components.

Isn't it possible to just alias an existing type and reference its stats function? Similar to this:

CREATE OR REPLACE FUNCTION customtype_in(cstring) RETURNS customtype AS

The only way to do this would be in C code to introduce a new stats kind entry with value more than 99 or 1000 or something. Not sure it's worth the hassle.

@nikkhils nikkhils merged commit 6db012d into timescale:master Oct 27, 2021
@nikkhils nikkhils deleted the analyze_crash branch October 27, 2021 09:08
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.

Segfault on analyse in multinode setup.
4 participants