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 crash with create_distributed_hypertable #1988

Merged
merged 1 commit into from Jun 17, 2020

Conversation

mkindahl
Copy link
Contributor

@mkindahl mkindahl commented Jun 12, 2020

Function ts_hypertable_create_from_info will error if the hypertable
already exists unless the if-not-exists flag is not set. If we reach
this point, either if-not-exists-flag was set or the hypertable did not
exist and was created above.

In ts_hypertable_create_from_info, a call to
ts_dimension_info_validate with space_dim_info will be made if (and
only if) the table did not exist. The function does not only validate
the dimension, it also set dimension_id field.

If the table already existed, created will be false and the
dimension_id will be set to the invalid OID, which means that
ts_hypertable_check_partitioning will crash since it it expect a
proper OID to be passed.

This commit fixes that by checking if the hypertable exists prior to
calling ts_hypertable_create_from_info in
ts_hypertable_create_internal and aborting with an error if the
hypertable already exists.

Fixes #1987

@mkindahl mkindahl requested a review from a team as a code owner June 12, 2020 12:42
@mkindahl mkindahl requested review from pmwkaa, WireBaron and gayyappan and removed request for a team June 12, 2020 12:42
src/hypertable.c Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jun 15, 2020

Codecov Report

Merging #1988 into master will increase coverage by 79.25%.
The diff coverage is 92.21%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1988       +/-   ##
===========================================
+ Coverage   10.91%   90.17%   +79.25%     
===========================================
  Files         205      211        +6     
  Lines       30185    33501     +3316     
===========================================
+ Hits         3296    30209    +26913     
+ Misses      26889     3292    -23597     
Flag Coverage Δ
#pr ?
Impacted Files Coverage Δ
src/bgw_policy/drop_chunks.c 96.72% <ø> (+87.79%) ⬆️
src/cache_invalidate.c 85.41% <ø> (+21.58%) ⬆️
src/catalog.c 86.58% <ø> (+19.91%) ⬆️
src/catalog.h 100.00% <ø> (ø)
src/chunk.h 100.00% <ø> (ø)
src/chunk_append/exec.c 95.84% <ø> (+95.84%) ⬆️
src/chunk_dispatch_state.c 95.29% <ø> (+95.29%) ⬆️
src/compat.h 100.00% <ø> (ø)
src/compression_with_clause.c 87.06% <ø> (+87.06%) ⬆️
src/constraint.c 69.56% <ø> (+69.56%) ⬆️
... and 292 more

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 a089843...898d155. Read the comment docs.

src/hypertable.c Outdated Show resolved Hide resolved
@mkindahl mkindahl requested a review from erimatnor June 15, 2020 09:09
src/hypertable.c Outdated Show resolved Hide resolved
@mkindahl mkindahl changed the title Read space dim info before checking partitioning Fix crash with create_distributed_hypertable Jun 16, 2020
@mkindahl mkindahl force-pushed the invalid_dimension_crash branch 2 times, most recently from 5581a05 to 83e007c Compare June 16, 2020 10:27
@mkindahl mkindahl requested a review from erimatnor June 16, 2020 11:46
Function `ts_hypertable_create_from_info` will error if the hypertable
already exists unless the if-not-exists flag is not set.  If we reach
this point, either if-not-exists-flag was set or the hypertable did not
exist and was created above.

In `ts_hypertable_create_from_info`, a call to
`ts_dimension_info_validate` with `space_dim_info` will be made if (and
only if) the table did not exist. The function does not only validate
the dimension, it also set `dimension_id` field.

 If the table already existed, `created` will be false and the
`dimension_id` will be set to the invalid OID, which means that
`ts_hypertable_check_partitioning` will crash since it it expect a
proper OID to be passed.

This commit fixes that by checking if the hypertable exists prior to
calling `ts_hypertable_create_from_info` in
`ts_hypertable_create_internal` and aborting with an error if the
hypertable already exists.

Fixes timescale#1987
@mkindahl mkindahl merged commit 5d18826 into timescale:master Jun 17, 2020
@mkindahl mkindahl deleted the invalid_dimension_crash branch June 17, 2020 06:38
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 when using if_not_exist on distributed hypertable
4 participants