Implement ML Features #381. SQLTransformer class and testcase

[ramanathanv]

Hi,
I am creating this draft pull request that implements the basic methods in SQLTransformer class along with Test cases.
This is related to "Implement ML Features" #381

[dnfadmin]

All CLA requirements met.

[ramanathanv]

Hi @GoEddie . I have implemented your review comments and have added TransformSchema() and GetStatement() in SQLTransformer class.

[ramanathanv]

Closed by mistake!

[GoEddie]

Looks good, a couple of small things

[Niharikadutta]

Added a few comments @ramanathanv . Thanks!

[ramanathanv]

@GoEddie , @Niharikadutta . Thank you for the review comments! I have implemented them in the commit now.

[Niharikadutta]

A few nits, but otherwise LGTM. Thanks @ramanathanv !

[ramanathanv]

Hi @GoEddie , @Niharikadutta . I have implemented the code review fixes. But I notice that the build fails and I guess it is not due to the code change. Can you please help me verify?

[Niharikadutta]

Hi @GoEddie , @Niharikadutta . I have implemented the code review fixes. But I notice that the build fails and I guess it is not due to the code change. Can you please help me verify?

@ramanathanv Yes this is a known issue that occurs intermittently and is being tracked in #773 . I have re-triggered this pipeline to re-run all the tests. Thanks!

[ramanathanv]

@ramanathanv Yes this is a known issue that occurs intermittently and is being tracked in #773 . I have re-triggered this pipeline to re-run all the tests. Thanks!

Thanks @Niharikadutta and @GoEddie . The build has passed all checks now. Please let me know if there is something pending from my end for this pull request to be accepted.

[GoEddie]

There is one comment left to complete (adding a short summary of the test) and then it looks good to me:

https://github.com/dotnet/spark/pull/781/files/4fcda8904d374c52fbb97dbf7618145d0a15ca4e#r535114596

[ramanathanv]

There is one comment left to complete (adding a short summary of the test) and then it looks good to me:

https://github.com/dotnet/spark/pull/781/files/4fcda8904d374c52fbb97dbf7618145d0a15ca4e#r535114596

Hi @GoEddie . I had included the comment earlier. But missed to "resolve" the review comment. I have now resolved it.

[GoEddie]

Hi @ramanathanv i'm not sure what happened to your changes but most of the comments have been resolved but without the code changes - maybe you have a local commit that you haven't pushed up to github? (or you made them on another branch or something?)

For example:

https://github.com/dotnet/spark/pull/781/files/4fcda8904d374c52fbb97dbf7618145d0a15ca4e#r535114596

The comment is: "nit: Can we add a small summary for this test?" but there is no summary, the ///

method headers

or:

https://github.com/dotnet/spark/pull/781/files/4fcda8904d374c52fbb97dbf7618145d0a15ca4e#r530571957

The code style says to use var only when calling a constructor, in this case where you call the constructor and then a method (TransformSchema) then we should use StructType instead of var

If you look through the other comments you will see a few things to implement - if you aren't sure what to do or need a hand with them let me know and I can help!

[ramanathanv]

Hi @GoEddie .
When I navigate to the "Files Changed" submenu within this PR, I notice that the review comments are indeed fixed. I have attached a screenshot along with this conversation. Can you please recheck and let me know if you are able to see the code with the review comments fixed?

[GoEddie]

@ramanathanv looks good!

[GoEddie]

@Niharikadutta could you review?

[imback82]

LGTM, thanks @ramanathanv!