Foreign copy from for pg_shardman #136
Conversation
…ardman. Also, callback args simplified.
That is, when 1) pg_pathman was compiled against postgres with shardman patches. 2) Shardman's COPY FROM was explicitly asked by setting renderzvous var. Also, check for 'freeze' option early, as before, to keep regression tests as they are.
Codecov Report
@@ Coverage Diff @@
## master #136 +/- ##
==========================================
+ Coverage 92.83% 92.83% +<.01%
==========================================
Files 28 28
Lines 5414 5418 +4
Branches 915 916 +1
==========================================
+ Hits 5026 5030 +4
Misses 388 388
Continue to review full report at Codecov.
|
| @@ -43,17 +43,15 @@ typedef struct | |||
| } ResultRelInfoHolder; | |||
|
|
|||
|
|
|||
| /* Forward declaration (for on_new_rri_holder()) */ | |||
| /* Forward declaration (for on_rri_holder()) */ | |||
There was a problem hiding this comment.
Is this a necessary change? This callback is intended to be called when a new ResultRelInfo holder is created.
| struct ResultPartsStorage; | ||
| typedef struct ResultPartsStorage ResultPartsStorage; | ||
|
|
||
| /* | ||
| * Callback to be fired at rri_holder creation. | ||
| * Callback to be fired at rri_holder creation/destruction. |
There was a problem hiding this comment.
IMHO there should be 2 callbacks: one for creation, and one for destruction. Why would we want to mix these roles?
| * Shut down FDWs. | ||
| */ | ||
| static void | ||
| finish_rri_copy(ResultRelInfoHolder *rri_holder, |
There was a problem hiding this comment.
I think this should be passed as a "delete callback". Maybe we should call them rri_init_callback & rri_fini_callback?
src/utility_stmt_hooking.c
Outdated
| */ | ||
| #ifndef PG_SHARDMAN | ||
| goto bail_out; /* to avoid 'unused label' warning */ | ||
| #else |
There was a problem hiding this comment.
This is the most disputable change so far. My idea's that we could create a struct of function pointers and allocate it using find_rendezvous_variable(). This would allow us to move the FDW-related logic to pg_shardman. The only drawback of such approach is that pg_shardman would have to include the same definition of such struct. I see several options.
Nice one, thanks! |
Main points:
COPY FROM ... FREEZE FALSEand copy would fail.