-
Notifications
You must be signed in to change notification settings - Fork 69
Foreign copy from for pg_shardman #136
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
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.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a necessary change? This callback is intended to be called when a new ResultRelInfo holder is created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO there should be 2 callbacks: one for creation, and one for destruction. Why would we want to mix these roles?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Small bug fixed: previously we could specify COPY FROM ... FREEZE FALSE and copy would fail.
Nice one, thanks!
Main points:
COPY FROM ... FREEZE FALSEand copy would fail.