-
Notifications
You must be signed in to change notification settings - Fork 22
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
Fix separation of columns #129
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm missing tests, not only on this change but also on other recent changes in the project.
I'm preparing a test for this case, which seems easy, but for other ongoing changes I'm missing C-level unit tests. @Algunenano any advice on how to approach them given your experience with PostGIS? |
What we do is we generate a static C library that doesn't depend on Postgresql and test that with unit tests, and then the final library is generated using that library and the files that depend on Postgres to generate the final .so that is distributed. |
I could not add a test with the pg fdw as I intended, since there is not pg we ignore, so I had to add a test for sql server instead. Tests for sql server are currently disabled in CI, so I ran it manually and, believe me, it succeeded. I'll open a separate ticket to enable back the sql server tests using the docker I used to run the test locally, but I'll leave this PR with the disabled new test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the extra effort!
OK, I had forgotten the disabled sql server tests runs on AppVeyor, and it has revealed a bug which wasn't observed in my sql server tests (which ran on linux). The bug, which should be fixed by e863ad1 was causing in this case the IMPORT FOREIGN SCHEMA to not import all remotes tables (the tables loop was terminated too early because the variable that controls it was being reused inside the loop. |
The comma-separation of columns was incorrect if the first column was skipped due to unsupported type (so a leading coman would appear:
,col1,col2
.