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

refactor: deprecate Fastpath API #903

Merged
merged 4 commits into from
May 4, 2018
Merged

Conversation

AlexElin
Copy link
Contributor

@AlexElin AlexElin commented Aug 3, 2017

change some methods in the Fastpath to accept varargs.

change some methods in the Fastpath to accept varargs.
*
* @param fnId Function id
* @param args FastpathArguments to pass to fastpath
* @return null if no data, byte[] otherwise
* @throws SQLException if a database-access error occurs.
*/
public byte[] fastpath(int fnId, FastpathArg[] args) throws SQLException {
public byte[] fastpath(int fnId, FastpathArg... args) throws SQLException {
Copy link
Member

Choose a reason for hiding this comment

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

I think this was somewhat deprecated API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

marked as deprecated

@codecov-io
Copy link

codecov-io commented Aug 3, 2017

Codecov Report

Merging #903 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #903      +/-   ##
============================================
+ Coverage     65.85%   65.86%   +<.01%     
- Complexity     3546     3547       +1     
============================================
  Files           166      166              
  Lines         15229    15230       +1     
  Branches       2470     2470              
============================================
+ Hits          10029    10031       +2     
  Misses         4022     4022              
+ Partials       1178     1177       -1

mark fastpath(int, FastpathArg[]) as Deprecated
@vlsi
Copy link
Member

vlsi commented Aug 4, 2017

Just for the reference (I mean the whole fastpath thing is obsolete, not just a particular method):

https://www.postgresql.org/docs/9.6/static/libpq-fastpath.html
This interface is somewhat obsolete, as one can achieve similar performance and greater functionality by setting up a prepared statement to define the function call. Then, executing the statement with binary transmission of parameters and results substitutes for a fast-path function call.

On the other hand, I've never performed a fastpath benchmark

@jorsol
Copy link
Member

jorsol commented Aug 4, 2017

Since is obsolete from upstream (it was obsolete since 7.4), I'm inclined to mark the whole fastpath in pgjdbc as deprecated.

I guess there is no need for a benchmark or for try to use varargs here.

deprecate Fastpath API
@AlexElin
Copy link
Contributor Author

AlexElin commented Aug 5, 2017

I marked the Fastpath API as deprecated and reverted using varargs

revert adding varargs to Fastpath
@AlexElin AlexElin changed the title refactor: add varargs in Fastpath refactor: deprecate Fastpath API Aug 7, 2017
@vlsi
Copy link
Member

vlsi commented Sep 25, 2017

+1

@AlexElin
Copy link
Contributor Author

AlexElin commented May 1, 2018

@vlsi can we get this PR merged?

@davecramer davecramer merged commit f8e21b6 into pgjdbc:master May 4, 2018
@bokken bokken mentioned this pull request Jul 3, 2018
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.

None yet

5 participants