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

add parameterised value support for text #75

Merged
merged 1 commit into from Feb 17, 2013

Conversation

zachaller
Copy link
Contributor

This allows support for text types to be parameters however TDS dose not allow text to be ouput parameters so that is not supported as well as I had some issue with empty string such as '' from my understanding it my not be supported (http://lists.ibiblio.org/pipermail/freetds/2010q2/025787.html) and if so the test case for it should either be removed or modified.

Signed-off-by: Zach Aller zachaller@hotmail.com

Signed-off-by: Zach Aller <zachaller@hotmail.com>
@pekim
Copy link
Collaborator

pekim commented Feb 17, 2013

Thanks for this. It's great to see the integration tests.

Regarding the null vs. empty string issue, I think that you may have uncovered a bug in the parsing of text values in tedious. A test added to datatypes-in-results-test.coffee fails with AssertionError: "" === null.

  exports.textEmpty = (test) ->
    execSql(test, "select cast('' as text) as text", '')

I can see that the response from the server is different from the one from this, passing, test.

  exports.textNull = (test) ->
    execSql(test, "select cast(null as text) as text", null)

So I think that it may be fixable. I'm digging in to the details of the response now.

@pekim pekim merged commit 48ea787 into tediousjs:master Feb 17, 2013
@pekim
Copy link
Collaborator

pekim commented Feb 17, 2013

Empty (zero-length) values for test/ntext/image columns were not being distinguished from null values.
I've fixed this in fd7ead7.

The failing test for an empty text parameter now passes.

Thanks again for this PR.

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

2 participants