Create index fails if hypertable has foreign table chunk #4899
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4899 +/- ##
==========================================
- Coverage 90.93% 90.79% -0.15%
==========================================
Files 225 225
Lines 52327 51533 -794
==========================================
- Hits 47586 46787 -799
- Misses 4741 4746 +5
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
gayyappan
requested review from
fabriziomello
and removed request for
svenklemm
| @@ -394,6 +394,14 @@ FROM chunk_view | |||
| WHERE hypertable_name = 'hyper_constr' | |||
| ORDER BY chunk_name; | |||
|
|
|||
| --Utility functions -check index creation on hypertable with OSM chunk | |||
There was a problem hiding this comment.
IMHO will be nice if you can improve a bit the comments here for people understand that we're skipping {CREATE|DROP} INDEX on OSM chunks...
There was a problem hiding this comment.
added more comments here.
src/process_utility.c
Outdated
| @@ -2494,6 +2495,14 @@ process_index_chunk_multitransaction(int32 hypertable_id, Oid chunk_relid, void | |||
| } | |||
| #endif | |||
|
|
|||
| chunk = ts_chunk_get_by_relid(chunk_relid, true); | |||
| if (IS_OSM_CHUNK(chunk)) /*cannot create index on foreign OSM chunk */ | |||
There was a problem hiding this comment.
What about add a NOTICE message where about skipping?
There was a problem hiding this comment.
IMO, any notice/error should be managed by OSM. This is an internal chunk and notice/error would not make sense to the end user.
There was a problem hiding this comment.
By moving chunk = ts_chunk_get_by_relid(chunk_relid, true); above you're breaking the lock semantics described below:
...
* Hold a lock on the hypertable index, and the chunk to prevent
* from being altered. Since we use the same relids across transactions,
...
There was a problem hiding this comment.
Thanks for catching the comment.I don't quite follow what it means
2514 * Hold a lock on the hypertable index, and the chunk to prevent
2515 * from being altered. Since we use the same relids across transactions,
2516 * there is a potential issue if the id gets reassigned between one
2517 * sub-transaction and the next. CLUSTER has a similar issue.
How does the id for a chunk get reassigned? Is this referring to chunk relid or chunk->fd.id or something else?
There was a problem hiding this comment.
Seems this is to prevent a concurrent session alter the same object (alter/drop columns, add/drop indexes, etc). Will be better @gayyappan to move your check and skip to after table_open and index_open
There was a problem hiding this comment.
@gayyappan is there any problems with moving this block back into the lock-protected section?
Patch: https://gist.github.com/zilder/a9b2b61af07361ae6a938b75ae516fb2
There was a problem hiding this comment.
yes, it is ok to move the check after the table_open. This code block will not work as-is. Note that we should clear state correctly before exiting from the function. Probably easiest with an if-else stmt.
There was a problem hiding this comment.
moved the code around to address the problem described by mfundul.
gayyappan
force-pushed
the
osm_index
branch
2 times, most recently
from
32589da
to
dd7b6c8
Compare
gayyappan
requested review from
zilder and
fabriziomello
and removed request for
mfundul
We cannot create indexes on foreign tables. This PR modifies process_index_chunk to skip OSM chunks
We cannot create indexes on foreign tables. This PR modifies process_index_chunk to skip OSM chunks
Disable-check: force-changelog-changed