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

Fixed SQL Server tables helpers #26

Merged
merged 1 commit into from
Apr 30, 2020

Conversation

pnetherwood
Copy link

Fixed SQL Server table helpers for foreign keys and references.

Fixed SQL Server table helpers for foreign keys and references.
@pnetherwood
Copy link
Author

I've sorted it out now. Even though sql server uses the information schema it does not match that of Postgress as first thought. I noticed the References query was not pulling in all of the table references so I've updated that to.

@kristijanhusak
Copy link
Owner

It works now, thanks!

@kristijanhusak kristijanhusak merged commit 3aed5cf into kristijanhusak:master Apr 30, 2020
@kristijanhusak
Copy link
Owner

@pnetherwood I added support for schemas and jumping to foreign key from the results buffer. I would appreciate if you would test it with your big databases just to make sure everything works fine.

@pnetherwood
Copy link
Author

I've just tried it and it worked fine on sql server.
I had a couple of problems on a mysql db. Rather than have my password in the DB URL I was waiting for a password prompt. Unfortunately it then failed to load the schemes and the only way to fix was to include the password in the URL. I then kept getting the message from mysql about it being insecure to use password on the command line. This extra message then caused the queries to return nothing in the preview window. I was all fixed once I went to using --login-path. The second issue is that the table helpers didn't work on schemas that were not the name of the selected database. For example once connected to my database I went to the INFORMATION_SCHEME and select List on COLUMN_NAMES and it came back with an error saying it couldn't find COLUMN_NAMES in my database.

@kristijanhusak
Copy link
Owner

All of these issues are on mysql?
Did you have them before introducing schemas?

@kristijanhusak
Copy link
Owner

I found an issue, and fixed it. Please do another test if possible.

@pnetherwood
Copy link
Author

That's fixed the password issue.
The schema issue remains. Say I have a URL of mysql://user@localhost/prod. When I log on I see prod listed in the schemas as well as another database e.g. dev. If I open List on dev.testdata I get an error saying Table prod.testdata does not exist.
I only have this problem with mysql and not sql server.

@pnetherwood
Copy link
Author

I got this when I tried the foreign key jump in sqlserver:

Error detected while processing function db_ui#dbout#jump_to_foreign_table[11]..db_ui#schemas#query:
line    1:
E716: Key not present in Dictionary: conn, 'interactive')
E116: Invalid arguments for function db#adapter#dispatch(a:db.conn, 'interactive')

Also, when I do a Refresh (R) tables the tree does not change to reflect the addition and deletion of tables.

@kristijanhusak
Copy link
Owner

I think i fixed both issues. Now schema is appended to the queries where applicable, and i introduced two variables, {schema}, and {optional_schema}.
Would appreciate if you would give it a test.
Also, for jump foreign key issue, i passed down wrong key, should be working now.

@pnetherwood
Copy link
Author

On the foreign key jump I get:

[DBUI] No valid foreign key found.

I was wondering if my foreign key query is returning the results in the form you are expecting.

Also the the refresh is still not updating the tree.

@kristijanhusak
Copy link
Owner

I'm using your query, just limiting results to 1 and getting two columns only.
Do you get the output for this query:

      SELECT TOP 1
         c2.table_name as foreign_table_name,
         kcu2.column_name as foreign_column_name
      from   information_schema.table_constraints c
             inner join information_schema.key_column_usage kcu
               on c.constraint_schema = kcu.constraint_schema
                  and c.constraint_name = kcu.constraint_name
             inner join information_schema.referential_constraints rc
               on c.constraint_schema = rc.constraint_schema
                  and c.constraint_name = rc.constraint_name
             inner join information_schema.table_constraints c2
               on rc.unique_constraint_schema = c2.constraint_schema
                  and rc.unique_constraint_name = c2.constraint_name
             inner join information_schema.key_column_usage kcu2
               on c2.constraint_schema = kcu2.constraint_schema
                  and c2.constraint_name = kcu2.constraint_name
                  and kcu.ordinal_position = kcu2.ordinal_position
      where  c.constraint_type = 'FOREIGN KEY'
      and kcu.column_name = '{column_name}'"

Just add your {column_name} in the query. If answer is yes, can you give me the full output of the query? Values itself are not important, so you can mask them if needed, I just need to know exact structure of output. Thanks.

@pnetherwood
Copy link
Author

I had to modify the query to include the table name and schema:

 and c.TABLE_NAME = '{table}' and c.TABLE_SCHEMA = '{schema}'

but then when I did I got a two column result.

@kristijanhusak
Copy link
Owner

Why you had to include those?

@pnetherwood
Copy link
Author

pnetherwood commented May 1, 2020 via email

@kristijanhusak
Copy link
Owner

I don't really know which schema and table is used in dbout buffer, that's why I rely only on column name. I don't expect it to return unique data, that's why I use top 1. Does it return anything for you only with column name and top 1 ?

@pnetherwood
Copy link
Author

Top 1 is not going to work unless the foreign key column name is unique across the whole database which is unlikely.
When you do the foreign key query, don't you have four columns: the FK name, original table, foreign table and foreign column which can then parse to do find the table name or am I missing something?

@kristijanhusak
Copy link
Owner

kristijanhusak commented May 1, 2020

Foreign key column is not unique, but the target key should be unique, at least in a sane database structure.
So with this query i'm getting first item where foreign key is what you are jumping from, and then getting the foreign column. So if you have a PersonID in a Posts table, when you call the mapping from Posts results person id cell, It will call above query that should return:

foreign_table_name | foreign_column_name
Persons            | ID 

From this I generate query select * from Persons where ID = 'cell value that was clicked from'
Here's a gif how it works for me, but I have only sample mssql database, not with the real data.
jump-foreign-key

@pnetherwood
Copy link
Author

Ah, I see how it works now. I was bringing up the foreign key constraint list and then trying to jump to the referenced table from the foreign key constraint. When I go to a table and jump from the from the row/column item it works great. Very nice. Apologies. I thought it was to jump from relationships not cells which explains why I was confused about the query. The gif really helped. Nice work.

Did you have chance to look at the refresh?

@kristijanhusak
Copy link
Owner

Yeah it's a bit hard to explain how it exactly works. This is how it's done in DBeaver, so i tried to simulate that.

What's exactly wrong with refresh? What is not updating?

@kristijanhusak
Copy link
Owner

Found it and fixed it. Should be good now.

@pnetherwood
Copy link
Author

pnetherwood commented May 2, 2020

I can't open the database from the tree. After the last update I'm getting:

[DBUI] Error connecting to db report: Vim(let):E688: More targets than List items

when I try to connect to the database. A normal dadbod DB command works fine with the same URL. My URL is in the form: sqlserver://user:password@host/db

@kristijanhusak
Copy link
Owner

I pushed a fix, but i'm confused how it happens.
If you run this query:

SELECT table_schema, table_name FROM information_schema.tables

Do you have some results where table_schema or table_name is empty?

@kristijanhusak
Copy link
Owner

I did few changes, hopefully there won't be any more issues. Let me know if you have any.

@pnetherwood
Copy link
Author

It logs on now and populates the schemas but the schemas are not populated with tables. The above query correctly returns the schemas and tables with no missing entries.

@pnetherwood
Copy link
Author

Ok. I've figured out what it is. With sqlcmd you can specify an ini file by exporting en env var SQLCMDINI=. (The ini file is quite handy as you cannot specify any additional parameters to sqlcmd in dadbod). In the file I had changed the separator to be a vertical line and I had specified mine as a unicode vertical line character. When I replaced it with a normal bar character it worked fine. So disregard the last message. It all works well now. Many thanks.

@pnetherwood
Copy link
Author

The refresh also all works fine too now. Thanks.
I really like that foreign key jumping feature (now that I understand it).

@kristijanhusak
Copy link
Owner

In latest commit I introduced using custom separator (pipe) for easier parsing, so maybe now it should work for you even with custom ini file. Not sure which one has precedence, sqlcmd argument or the ini file.

@pnetherwood
Copy link
Author

The ini file appears to have precedence and I have set it to the pipe character too.
What I'm using it for is to make the columns in the results pane look nicer e.g.

name           |description                                                 |countryCode|marketIdentifierCode|iceberg|crossTrade|supportsCancelReplace|MIC 
---------------|------------------------------------------------------------|-----------|--------------------|-------|----------|---------------------|----
BMF            |Bolsa de Mercadorias e Futuros                              |BRL        |BVMF                |N      |NO        |N                    |NULL
CBOE           |Chicago Board Options Exchange                              |USA        |XCBO                |N      |0         |Y                    |NULL
CBOT           |Chicago Board of Trade                                      |USA        |XCBT                |Y      |5         |Y                    |NULL
CFFEX          |China Financial Futures Exchange                            |CHN        |???                 |N      |0         |N                    |NULL
CME            |Chicago Mercantile Exchange                                 |USA        |XCME                |Y      |5         |Y                    |NULL

But now that messes with your foreign query jump functionality because you are expecting a space delimiter on the column name in dbout s:get_cell_range. Any suggestions? I could remove the separator in the ini file but I rather like it. Could you rely on naming conventions instead of a delimiter?

@kristijanhusak
Copy link
Owner

I changed the way i'm getting the range. As long as delimiter is not set to -, which is the same char used for the table line, it should work.

@pnetherwood
Copy link
Author

Nice. That works great.

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