search

keboola / db-extractor-mssql

MIT License
1 stars 2 forks source link

getSimplePdoQuery and simpleQuery #132

Closed tomasfejfar closed 5 years ago

tomasfejfar commented 5 years ago

Does almost the same thing, don't you think?

https://github.com/keboola/db-extractor-mssql/blob/master/src/Keboola/DbExtractor/Extractor/MSSQL.php#L460-L502

https://github.com/keboola/db-extractor-mssql/blob/master/src/Keboola/DbExtractor/Extractor/MSSQL.php#L401-L435

pivnicek commented 5 years ago

Yes, it wasn't ideally split up. The difference is in the column handling https://github.com/keboola/db-extractor-mssql/blob/master/src/Keboola/DbExtractor/Extractor/MSSQL.php#L418 We can definitely reduce it to a single method and conditionally write out the columns

pivnicek commented 5 years ago

@tomasfejfar just to confirm, shall I go ahead with this simple refactor?

tomasfejfar commented 5 years ago

I think you can. CDATA is long way from released and we'll probably still be dealing with MSSQL-ex for a long time. It makes sense to simplify...

pivnicek commented 5 years ago

So, https://github.com/keboola/db-extractor-mssql/pull/133 is my dumb attempt at this, but I'm not so sure that it's so helpful. May be best to combine it with a more thorough separation as in https://github.com/keboola/db-extractor-mssql/issues/71