search

ibmdb / node-ibm_db

IBM DB2 and IBM Informix bindings for node
MIT License
188 stars 151 forks source link

queryResult deferred callback result changed with version 3.2.3 #979

Closed Stoyz closed 5 months ago

Stoyz commented 5 months ago

Hello, I am opening this issue to ask a question about the PR #953 (merged and published in version 3.2.3) which changes the deferred resolve of the queryResult function to an array as you can see in this comparison.

Has the change been intentional? If so, do you mind if I ask why the change has not been marked as breaking change?

I have a piece of code which does not work anymore due to the change and I had to take the first value of the array in order to use the getAffectedRowsSync function.

Follows a portion of the code I am now using:

const result = await connection.queryResult(sql, params);

// the following line used to be: const affectedRows = result.getAffectedRowsSync();
const affectedRows = result[0].getAffectedRowsSync();

Thank you.

bimalkjha commented 5 months ago

@Stoyz I think you are pointing about below change in PR: image

This change has fixed a bug in the queryResult() API. await result= conn.queryResult() do not return outparams for Stored Procedure call with OUTPUT or INOUT type of parameters, when async-await programming is used. So, we are resolving the promise as an array and data can be extracted from the array by application. We found this bug in the code while fixing other issues. So, the change is intentional as a bug fix. Hope, it clarifies. Thanks.

Stoyz commented 5 months ago

Thank you for the clarification @bimalkjha

Only thing, in my opinion, this change should've been marked as breaking because it broke my code and there was no mention about this.

bimalkjha commented 5 months ago

@Stoyz I agree, but I am not sure about how to mark about it. Could you please help me with your suggestion about the steps or place to mention about it? We'll try to follow it. Thanks.

Stoyz commented 5 months ago

@bimalkjha sure thing.

In my opinion and in the majority of libraries, a common place where to tell a user if a change is breaking or not is in the changelog file. In your case the file is CHANGES.md in which you can just write in parenthesis that a change is breaking.

Follows an example of a breaking change in markdown:

* (__breaking__) change: some thing has changed in issue https://github.com/ibmdb/node-ibm_db/issues/960

In addition to this, when there is a breaking change a major release should occur if you follow the Semantic Versioning guidelines.

Hope to have been helpful.

bimalkjha commented 5 months ago

Thanks @Stoyz . Will update Changes.md in future breaking changes accordingly and do major release. Thanks.