tds-fdw / tds_fdw

A PostgreSQL foreign data wrapper to connect to TDS databases (Sybase and Microsoft SQL Server)
Other
381 stars 102 forks source link

Import foreign schema #91

Closed za-arthur closed 8 years ago

za-arthur commented 8 years ago

Hello,

This pull request fixes the issue #75 I added tests and some documentation. I added test postgresql/003_import_schema.sql, so other tests was renamed.

If you have notes about this I will fix it.

jenkins-juliogonzalez commented 8 years ago

Can one of the admins verify this patch?

GeoffMontee commented 8 years ago

Test this, please

jenkins-juliogonzalez commented 8 years ago

Test FAILed.

juliogonzalez commented 8 years ago

As IMPORT FOREIGN SCHEMA is only available for PostgreSQL >= 9.5, I will need to modify the script and the tests to support different PostgreSQL versions.

@select-artur, let me think a little about it and I'll make a patch so you can rebase it into this branch. It won't take long.

Meanwhile, may I suggest you squash your commits into a single one?

za-arthur commented 8 years ago

Meanwhile, may I suggest you squash your commits into a single one?

Done.

@select-artur, let me think a little about it and I'll make a patch so you can rebase it into this branch. It won't take long.

Thank you.

juliogonzalez commented 8 years ago

@select-artur, the patch is ready and merged.

Now all the SQL files have a corresponding JSON file with, among other things, the minimum and maximum version required for each of the tests

Please rebase changes from master into your branch, fix the conflicts and just add a file tests/tests/postgresql/003_import_schema.json with the following content:

{
    "test_desc" : "foreign schema import",
    "server" : {
        "version" : {
            "min" : "9.5.0",
            "max" : ""
        }
    }
}

As the foreign tables for all other tests are deleted before running the actual test (check tests/tests/postgresql/003_tinyintmin.sql for example), you could also create your test as number 032, to avoid renaming all the previous tests (if it's more comfortable for you)

juliogonzalez commented 8 years ago

I see that there are no conflicts, so it should be enough if you just add the JSON file as "tests/tests/postgresql/003_import_schema.json" :-)

za-arthur commented 8 years ago

Thanks! I will add it.

juliogonzalez commented 8 years ago

Great! @GeoffMontee or me will run the tests as soon as one of us see the commit.

za-arthur commented 8 years ago

I suppose I did it.

you could also create your test as number 032, to avoid renaming all the previous tests (if it's more comfortable for you)

I've create import test as number 003 because I concern if IMPORT FOREIGN SCHEMA will be executed by the last query, then an error can be throwed. Because all foreign table will be exist. I cant test it now, because I will have the MS SQL environment only on monday.

Of course I can create import test as number 032 if I'm wrong.

GeoffMontee commented 8 years ago

Test this, please

juliogonzalez commented 8 years ago

@select-artur, good point!

I can't see any mention about what will happen if one of the tables already exist, so most probably it will fail as you correctly guessed.

jenkins-juliogonzalez commented 8 years ago

Test FAILed.

juliogonzalez commented 8 years ago

retest this, please (test faileds because of a problem starting PostgreSQL, not because of the PR itself)

jenkins-juliogonzalez commented 8 years ago

Test PASSed.

GeoffMontee commented 8 years ago

The tests passed, so this has been merged. Thanks a lot for the contribution, @select-artur!

za-arthur commented 8 years ago

Thanks!

ellmout commented 8 years ago

Hi,

Not working on Sybase (12.5), since Sybase use sysobjects to store Tables informations.

za-arthur commented 8 years ago

Hello @elliotvr , unfortunately I haven't Sybase server instance. So I cant implement and test it.

Maybe the solution is to add new option to IMPORT FOREIGN SCHEMA. Depening on this option it is necessary use various queries to retreive information about objects. Or maybe there is a possibility to get from tds_fdw is server Sybase or SQL Server.

GeoffMontee commented 8 years ago

Hi @elliotvr,

I just created issue #104 to track that we do not have a Sybase implementation of IMPORT FOREIGN SCHEMA yet.

@select-artur,

If you are interested in trying to develop the Sybase implementation of IMPORT FOREIGN SCHEMA as well, it looks like there is a free version of Sybase ASE available called SAP Adaptive Server Enterprise Express Edition. (Note: Sybase was purchased by SAP, so the name of the software has changed.)

za-arthur commented 8 years ago

Hi, @GeoffMontee, I think I could develop it in a couple weeks.

GeoffMontee commented 8 years ago

That would be awesome, @select-artur! tds_fdw's Sybase users would probably appreciate having that functionality. Thanks!

za-arthur commented 8 years ago

@elliotvr can you investigate the pull request #108 ?

It seems that there is not schemas in SAP ASE. So IMPORT FOREIGN SCHEMA imports tables of specific owner. So for SAP ASE table owner is equal to schema. Isn't wrong?

GeoffMontee commented 8 years ago

@elliotvr,

PR #108 from @select-artur has just been merged into the master branch if you want to test it out.

Thanks again for the great patch, @select-artur!