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

Consistent Column count with null values across resultset #26

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gkarthiks
Copy link

@gkarthiks gkarthiks commented Jan 11, 2018

Issue Reference: #25

Description:

The previous version was ignoring the columns having null values for any records at any position.

So for example, if the first row has null for column number 2, then that column will not be added. If the next row has null for 3rd column and that will not be added in the JSON resulting in inconsistent column counts across the result set JSON.

  1. Adding the attribute allowNull as an optional parameter to include null valued columns in the response JSON for maintaining the consistent column count across rows. If not passed, then the driver will work as earlier. No need to change anything, already backward compatible.
  2. Added the auto created JavaDoc files.

TODOs:

  • Ready to review
  • Ready to merge

@@ -35,7 +41,7 @@ function Sybase(host, port, dbname, username, password, logTiming, pathToJavaBri
Sybase.prototype.connect = function(callback)
{
var that = this;
this.javaDB = spawn('java',["-jar",this.pathToJavaBridge, this.host, this.port, this.dbname, this.username, this.password]);
this.javaDB = spawn('java',["-jar",this.pathToJavaBridge, this.host, this.port, this.dbname, this.username, this.password, this.allowNull]);
Copy link

@ktordoff13 ktordoff13 Jan 28, 2020

Choose a reason for hiding this comment

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

I am getting an error when allowNull is set to true
Error: Expecting the arguments: host, port, dbname, username, password

@theneva
Copy link
Collaborator

theneva commented Jan 31, 2020

Hi! This repo is no longer maintained (nobody here has access to a Sybase instance to test it anymore), so I'm afraid this is pretty unlikely to get merged.

@cezarcatarin
Copy link

Oi! Este repo não é mais mantido (ninguém aqui tem acesso a uma instância do Sybase para testá-lo), então temo que seja muito improvável que seja mesclado.

I'm having this same problem, can anyone accept the pull request? can i help to test

@lroal
Copy link

lroal commented Sep 22, 2021

I am having the same problem. This is a big show stopper.

@gkarthiks
Copy link
Author

If anyone is ready to accept this, I can rebase and resolve conflicts.

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.

5 participants