Closed bgamari closed 8 years ago
It would be nice to throw an exception other than the default ErrorCall
exception, but it seems this is a step in the right direction anyway.
Also, it would be nice to have a demo of several failure modes of COPY IN, possibly as part of the test suite, though I think that's slightly less important than the comment above.
Leon P Smith notifications@github.com writes:
Also, it would be nice to have a demo of several failure modes of COPY IN, possibly as part of the test suite, though I think that's slightly less important than the comment above.
I actually have such a thing locally. I'll try to clean it up and integrate it into the testsuite.
Hmm, what's the testing strategy where you commit some expected output, then you can simply inspect the diffs as you work on the code? That seems like a good strategy here, though I'm certainly open to other possibilities.
This issue made me think of #181, which seems at least somewhat related. I had a small bit of difficulty finding that issue; I didn't remember that it was a PR, that it was recent, and that it was you!
Leon P Smith notifications@github.com writes:
Hmm, what's the testing strategy where you commit some expected output, then you can simply inspect the diffs as you work on the code? That seems like a good strategy here, though I'm certainly open to other possibilities.
I assume you mean the approach taken by
tasty-golden
? If so then sure, that sounds sensible. In that case would you accept a dependency on tasty-golden?
Leon P Smith notifications@github.com writes:
This issue made me think of #181, which seems at least somewhat related. I had a small bit of difficulty finding that issue; I didn't remember that it was a PR, that it was recent, and that it was you!
Indeed, my trouble with #181 was in part that the underlying issue was being masked by the lack of a sensible error message.
Sure, having the test suite depend on tasty-golden sounds fine to me. (Also, I'm less picky about test suite dependencies.)
Also, I'm not that surprised that this is the fix that was needed in #181, it was something vaguely along these lines I was sort of expecting in the first place.
This ensures that the exception thrown includes the cause of the error. Sadly it seems that the only way to determine that a COPY IN has failed is to fail to parse the row count (as the result status is typically an empty string. It seems that all of the getResults calls after the putCopyEnd command inexplicably return success, despite the fact that something failed.