CartoDB / odbc_fdw

PostgreSQL Foreign-data Wrapper for ODBC
Other
66 stars 22 forks source link

Fix separation of columns #129

Closed jgoizueta closed 3 years ago

jgoizueta commented 3 years ago

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.

jgoizueta commented 3 years ago

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?

Algunenano commented 3 years ago

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.

jgoizueta commented 3 years ago

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.

jgoizueta commented 3 years ago

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.