Views related fixes for structure dump and load #400
Conversation
|
If the silence is simply because you guys are too busy, that's fine, and I apologise for bugging you. If it's more than you don't understand what or why I required this patch I can provide some additional information with practical examples. If you don't like the patch in it's current form, and I to need alter how it works I can do so. If it's a case of you don't want to accept it (with or without reason) I'd just rather know so I can plan for the future :) |
|
@theangryangel Sorry for the silence, been super busy lately. Taking a look at this now. |
| # Also create view needs to be the first operation in the batch. | ||
| File.open(filename, 'a') { |file| | ||
| connection.select_all("select definition, o.type from sys.objects as o join sys.sql_modules as m on m.object_id = o.object_id where o.type = 'V'").each do |row| | ||
| file.puts "\r\nGO\r\n#{row['definition']}" |
There was a problem hiding this comment.
Why use this SQL vs our existing view_information method? Does one work better than the other?
|
@theangryangel Made a few comments. This looks great and I can add it. Can you take a look at some of my comments first? Also, do you feel up to adding a test to the |
|
No worries @metaskills - thanks for getting back to me, I didn't mean to come across as shitty - I'm finding a pattern of pull requests and patches getting ignored on various projects at the moment and I'd rather not spend what time I have on projects that won't accept them. I'll adjust my timescale expectations accordingly for this one 😄 The view_information I wasn't aware existed tbh. I did have a look but I must've missed it. I'll put aside some time over the next day or two and switch to that if it's just as good/better, and I'll add some tests at the same time as well :) |
|
SWEET... can't wait... I very much appreciate the efforts too. A welcome change and I'll make sure we get this pulled in. |
…w simple tests to ensure that they are all working as expected.
|
Sorry for the delay in this one. I got a bit sidetracked with another project. I've updated the branch to use On my test box all tests pass, with exception to a UUID one, but I've not touched that one, so I'm not sure if it was always broken. |
|
Just writing to let you know I am still holding on this. I had a recent trip to Microsoft and I think we may have a cross platform native way to dump a DB that fixes all of this. If so, the adapter will be moving to it vs devncopy. |
|
The issue I have with defncopy is that it seems to generate the DDL itself (unless I'm being a complete idiot). This requires defncopy to be updated with each new column type (https://github.com/FreeTDS/freetds/blob/ea64a79533dad44c717d7b9764d6d97c8f925c58/src/apps/defncopy.c#L478). By utilizing SQL server to provide definitions of the objects itself, this mitigates the problem of keeping defncopy up to date. That said, I believe this patch does need an additional update as I believe that I found large definitions were truncated with the original query I used. I'd need to double check though. Just incase things have been forgotten over time, this patch also introduces alterations to the structure:load functionality, which splits the structure.sql by "GO" which allows for the file to have elements which isn't address by FreeTDS. I'm happy to update as necessary, if it's worth my time. |
|
I would hold off for a bit. Will repot back in a few days.. |
|
Re-opening. Not ignoring this, just have not got to it yet. |
|
Looking at the progress with #437 I assumed it was irrelevant, at least in-part, tbh 😄 |
|
OK, after finally getting back into things, I would like to put the energy into #437. Apologies but please continue to use your branch of this work as needed. I will be doing some work in the 4-2-stable branch to get it running on SQL Server 2016 and running tests on TravisCI/CircleCI too. I believe finding a way to either allow development to happen on Windows/LINUX hosts which can run SQL Server and shell out to |
This patch addresses 2 issues I have with structure dumping and loading:
I'm not sure if these are the "correct" fixes, but it seems to be working for me at present. I'm happy to refactor as required.
As an aside I'm also having an issue with defncopy not pulling through auto-generating identities correctly. if you guys are happy with this pull request and don't tell me not to I'll be creating a new patch that completely removes defncopy from the equation to fix the identities problem.