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

INPUT_OUTPUT isn't valid #19

Closed
abmusse opened this issue Oct 4, 2018 · 9 comments
Closed

INPUT_OUTPUT isn't valid #19

abmusse opened this issue Oct 4, 2018 · 9 comments
Labels
bug Something isn't working

Comments

@abmusse
Copy link
Member

abmusse commented Oct 4, 2018

Original report by Aaron Bartell (Bitbucket: aaronbartell, GitHub: aaronbartell).


The docs say either SQL_PARAM_INPUT_OUTPUT or INPUT_OUTPUT can be used but it appears only SQL_PARAM_INPUT_OUTPUT works. See below for how to reproduce the error.

$]› node
> const db = require('idb-pconnector');
undefined
> console.log(db.SQL_PARAM_INPUT_OUTPUT);
3
undefined
> console.log(db.INPUT_OUTPUT);
undefined
undefined
@abmusse
Copy link
Member Author

abmusse commented Oct 5, 2018

@aaronbartell Good Catch that is a typo in the docs. currently should be PARAM_INPUT_OUTPUT.

What are your thoughts on these aliases?

#!js
//alias variables
exports.BIND_CLOB = idb.BIND_CLOB;
exports.BIND_STRING = idb.BIND_STRING;
exports.BIND_INT = idb.BIND_INT;
exports.BIND_NULL = idb.BIND_NULL;
exports.BIND_NUMERIC = idb.BIND_NUMERIC;
exports.BIND_BOOLEAN = idb.BIND_BOOLEAN;
exports.BIND_BINARY = idb.BIND_BINARY;
exports.BIND_BLOB = idb.BIND_BLOB;
exports.PARAM_INPUT = idb.SQL_PARAM_INPUT;
exports.PARAM_OUTPUT = idb.SQL_PARAM_OUTPUT;
exports.PARAM_INPUT_OUTPUT = idb.SQL_PARAM_INPUT_OUTPUT;

Should we rename PARAM_INPUT_OUTPUT to PARAM_INOUT

@abmusse
Copy link
Member Author

abmusse commented Oct 5, 2018

Original comment by Aaron Bartell (Bitbucket: aaronbartell, GitHub: aaronbartell).


What are your thoughts on these aliases?

If the goal is succinctness then I'd do the following. Note I haven't checked for collisions (i.e. whether there is a PARAM and BIND that would both be OUT, for example).

Also, I called out BIND_STRING below. I don't believe that is a valid DB2 datatype. I get that it might have been created to make it more language friendly, but it is the one datatype that threw me off and I had to research how to use it by looking at the raw code in the repo. In short, it had the exact opposite effect (instead of simplifying it made my implementation take longer). I'd instead stick to CHAR.

Those are my opinions :-)

exports.CLOB = idb.BIND_CLOB;
exports.STRING = idb.BIND_STRING;   <-------
exports.INT = idb.BIND_INT;
exports.NULL = idb.BIND_NULL;
exports.NUMERIC = idb.BIND_NUMERIC;
exports.BOOLEAN = idb.BIND_BOOLEAN;
exports.BINARY = idb.BIND_BINARY;
exports.BLOB = idb.BIND_BLOB;
exports.IN = idb.SQL_PARAM_INPUT;
exports.OUT = idb.SQL_PARAM_OUTPUT;
exports.IN_OUT = idb.SQL_PARAM_INPUT_OUTPUT;

@abmusse
Copy link
Member Author

abmusse commented Oct 8, 2018

@aaronbartell I like it! the goal is make it easy to use but at the same time understandable.
Instead of IN_OUT I think we should go with INOUT or BOTH.

I'm considering scrapping the current aliases and going with these.

#!js

//alias variables
// exports.BIND_CLOB = idb.BIND_CLOB;
// exports.BIND_STRING = idb.BIND_STRING;
// exports.BIND_INT = idb.BIND_INT;
// exports.BIND_NULL = idb.BIND_NULL;
// exports.BIND_NUMERIC = idb.BIND_NUMERIC;
// exports.BIND_BOOLEAN = idb.BIND_BOOLEAN;
// exports.BIND_BINARY = idb.BIND_BINARY;
// exports.BIND_BLOB = idb.BIND_BLOB;
// exports.PARAM_INPUT = idb.SQL_PARAM_INPUT;
// exports.PARAM_OUTPUT = idb.SQL_PARAM_OUTPUT;
// exports.PARAM_INPUT_OUTPUT = idb.SQL_PARAM_INPUT_OUTPUT;
exports.CLOB = idb.BIND_CLOB;
exports.STRING = idb.BIND_STRING;
exports.INT = idb.BIND_INT;
exports.NULL = idb.BIND_NULL;
exports.NUMERIC = idb.BIND_NUMERIC;
exports.BOOLEAN = idb.BIND_BOOLEAN;
exports.BINARY = idb.BIND_BINARY;
exports.BLOB = idb.BIND_BLOB;
exports.IN = idb.SQL_PARAM_INPUT;
exports.OUT = idb.SQL_PARAM_OUTPUT;
exports.INOUT = idb.SQL_PARAM_INPUT_OUTPUT;

@abmusse
Copy link
Member Author

abmusse commented Oct 8, 2018

Original comment by Aaron Bartell (Bitbucket: aaronbartell, GitHub: aaronbartell).


@abmusse, I like what you've proposed.

@abmusse
Copy link
Member Author

abmusse commented Oct 8, 2018

@aaronbartell Nice! I will push up the changes upstream.

@abmusse
Copy link
Member Author

abmusse commented Oct 8, 2018

@aaronbartell Now that I think about it, I think we should only expose this set of constants

exports.CLOB = idb.BIND_CLOB;
exports.STRING = idb.BIND_STRING;
exports.INT = idb.BIND_INT;
exports.NULL = idb.BIND_NULL;
exports.NUMERIC = idb.BIND_NUMERIC;
exports.BOOLEAN = idb.BIND_BOOLEAN;
exports.BINARY = idb.BIND_BINARY;
exports.BLOB = idb.BIND_BLOB;
exports.IN = idb.SQL_PARAM_INPUT;
exports.OUT = idb.SQL_PARAM_OUTPUT;
exports.INOUT = idb.SQL_PARAM_INPUT_OUTPUT;

and remove this set


SQL_BIND_CHAR
SQL_BIND_INT
SQL_BIND_NUMERIC
SQL_BIND_BINARY
SQL_BIND_BLOB
SQL_BIND_CLOB
SQL_BIND_BOOLEAN
SQL_BIND_NULL_DATA

In our docs we would state:

IN/OUT TYPE CAN BE:
- IN
- OUT
- INOUT
          
INDICATORS CAN BE:
- STRING
- INT
- NUMERIC
- BINARY
- BLOB
- CLOB
- BOOLEAN
- NULL

Makes sense to only have what we need. And with the new major version update to 1.0.0 this is the time to straighten it out.

@abmusse
Copy link
Member Author

abmusse commented Oct 8, 2018

Original comment by Aaron Bartell (Bitbucket: aaronbartell, GitHub: aaronbartell).


Looks good. I would still include exports.CHAR as that is a native (and very common) SQL type.

@abmusse
Copy link
Member Author

abmusse commented Oct 8, 2018

@aaronbartell sounds good will be:

#!js

exports.CLOB = idb.BIND_CLOB;
exports.STRING = idb.BIND_STRING;
exports.CHAR = idb.BIND_STRING;
exports.INT = idb.BIND_INT;
exports.NULL = idb.BIND_NULL;
exports.NUMERIC = idb.BIND_NUMERIC;
exports.BOOLEAN = idb.BIND_BOOLEAN;
exports.BINARY = idb.BIND_BINARY;
exports.BLOB = idb.BIND_BLOB;
exports.IN = idb.SQL_PARAM_INPUT;
exports.OUT = idb.SQL_PARAM_OUTPUT;
exports.INOUT = idb.SQL_PARAM_INPUT_OUTPUT;

@abmusse
Copy link
Member Author

abmusse commented Oct 10, 2018

Resolved with PR #17

@abmusse abmusse closed this as completed Oct 10, 2018
@abmusse abmusse added minor bug Something isn't working labels Jan 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant